Skip to content

Conversation

@DaveCTurner
Copy link
Contributor

A completely idle transport_worker thread is reported as 0.0% idle, which is confusing. Moreover the docs on the network threading model do not reflect the changes made in #90482. This commit fixes both of those things.

A completely idle `transport_worker` thread is reported as `0.0%` idle,
which is confusing. Moreover the docs on the network threading model do
not reflect the changes made in elastic#90482. This commit fixes both of those
things.
@DaveCTurner DaveCTurner added >non-issue :Core/Infra/Core Core issues without another label v8.9.0 labels May 24, 2023
@github-actions
Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label May 24, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@DaveCTurner
Copy link
Contributor Author

The docs in 8.6/8.7/8.8 are similarly outdated; I'll open a PR to fix them up too once this is merged.

double percentCpu = getTimeSharePercentage(topThread.getCpuTime());
double percentOther = getTimeSharePercentage(topThread.getOtherTime());
double percentOther = Transports.isTransportThread(threadName) && topThread.getCpuTime() == 0L
? 100.0
Copy link
Member

Choose a reason for hiding this comment

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

Should this be 100 - topThread.getCpuTime? Like, is all the stuff no in cpuTime idle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmm kinda, but (a) the cpu time is zero on this branch so subtracting it makes no difference, and (b) if we made this change on the nonzero-cpu-time branch then we would lose the ability to spot cases where the thread was actually not RUNNABLE for some time (e.g. waiting on a mutex, which is not what we consider to be "idle" here).

Copy link
Member

Choose a reason for hiding this comment

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

Ah! I see. There's other stuff not in getCpuTime. Makes sense.

@DaveCTurner DaveCTurner merged commit 2a49ad9 into elastic:main May 25, 2023
@DaveCTurner DaveCTurner deleted the 2023-05-24-hot-threads-transport-workers branch May 25, 2023 11:08
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 25, 2023
Backports the docs changes from elastic#96315, reflecting the change introduced
in elastic#90482, but adjusting them slightly to reflect the different
behaviour in earlier versions.
elasticsearchmachine pushed a commit that referenced this pull request May 25, 2023
Backports the docs changes from #96315, reflecting the change introduced
in #90482, but adjusting them slightly to reflect the different
behaviour in earlier versions.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 25, 2023
Backports the docs changes from elastic#96315, reflecting the change introduced
in elastic#90482, but adjusting them slightly to reflect the different
behaviour in earlier versions.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 25, 2023
Backports the docs changes from elastic#96315, reflecting the change introduced
in elastic#90482, but adjusting them slightly to reflect the different
behaviour in earlier versions.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elasticsearchmachine pushed a commit that referenced this pull request May 25, 2023
Backports the docs changes from #96315, reflecting the change introduced
in #90482, but adjusting them slightly to reflect the different
behaviour in earlier versions.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elasticsearchmachine pushed a commit that referenced this pull request May 25, 2023
Backports the docs changes from #96315, reflecting the change introduced
in #90482, but adjusting them slightly to reflect the different
behaviour in earlier versions.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants