Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Note in docs about interpreting IO stats when running in docker #113676

Merged

Conversation

lukewhiting
Copy link
Contributor

This PR adds documentation describing possible inaccuracies in the FS statistics from the _nodes/stats endpoint when running inside a container

@lukewhiting lukewhiting added the :Data Management/Stats Statistics tracking and retrieval APIs label Sep 27, 2024
Copy link
Contributor

Documentation preview:

@lukewhiting lukewhiting added the >docs General docs changes label Sep 27, 2024
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team v9.0.0 Team:Docs Meta label for docs team labels Sep 27, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@lukewhiting lukewhiting added auto-backport-and-merge Automatically create backport pull requests and merge when ready v8.16.0 and removed Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team v9.0.0 labels Sep 27, 2024
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team labels Sep 27, 2024
@lukewhiting lukewhiting added v9.0.0 and removed Team:Docs Meta label for docs team labels Sep 27, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Docs Meta label for docs team label Sep 27, 2024
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better to use more neutral wording here. The stats we expose aren't inaccurate, they are an accurate reflection of something that might not be exactly what you expect.

Maybe we should call out explicitly that these stats come from /proc/diskstats and leave it up to the user to worry about whether that's what they want or not.

docs/reference/cluster/nodes-stats.asciidoc Outdated Show resolved Hide resolved
@lukewhiting
Copy link
Contributor Author

lukewhiting commented Sep 27, 2024

I think it'd be better to use more neutral wording here. The stats we expose aren't inaccurate, they are an accurate reflection of something that might not be exactly what you expect.

Maybe we should call out explicitly that these stats come from /proc/diskstats and leave it up to the user to worry about whether that's what they want or not.

I think you are right about more natural language but I do want to try and highlight this specific example as not all users may be immediately familiar with /proc/diskstats and the implications of accessing it within a container.

How about:

NOTE: These statistics are derived from `proc/diskstats` and represent the underlying host
filesystem even when running within a container therefore extra care is needed to ensure
they are correctly interpreted in such circumstances.

@lukewhiting lukewhiting changed the title Note in docs about incorrect IO stats when running in docker Note in docs about interpreting IO stats when running in docker Sep 27, 2024
@DaveCTurner
Copy link
Contributor

Nit: these stats relate to block devices rather than filesystems. Also I'd rather avoid the passive voice ("care is needed", "are correctly interpreted"). How about this?

NOTE: These statistics are derived from the `/proc/diskstats` kernel interface.
This interface accounts for IO performed by all processes on the system, even
if you are running {es} within a container.

@lukewhiting
Copy link
Contributor Author

Looks good. Have made that change. Thanks for the assist!

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

docs/reference/cluster/nodes-stats.asciidoc Outdated Show resolved Hide resolved
@lukewhiting lukewhiting merged commit db632ee into elastic:main Sep 27, 2024
5 checks passed
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

lukewhiting added a commit to lukewhiting/elasticsearch that referenced this pull request Sep 27, 2024
…tic#113676)

* Note in docs about incorrect IO stats when running in docker

* Update docs/reference/cluster/nodes-stats.asciidoc

Co-authored-by: David Turner <[email protected]>

* Requested PR changes to wording

* Update docs/reference/cluster/nodes-stats.asciidoc

Co-authored-by: David Turner <[email protected]>

---------

Co-authored-by: David Turner <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Sep 27, 2024
) (#113685)

* Note in docs about incorrect IO stats when running in docker

* Update docs/reference/cluster/nodes-stats.asciidoc



* Requested PR changes to wording

* Update docs/reference/cluster/nodes-stats.asciidoc



---------

Co-authored-by: David Turner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready :Data Management/Stats Statistics tracking and retrieval APIs >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_nodes/stats can return host filesystem stats when running in a container
3 participants