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

Add total wait time to thread pool in nodes stats #5120

Merged
merged 9 commits into from
Oct 16, 2023
Merged

Conversation

kolchfa-aws
Copy link
Collaborator

Fixes #5007

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@kolchfa-aws kolchfa-aws self-assigned this Sep 29, 2023
@kolchfa-aws kolchfa-aws added release-notes PR: Include this PR in the automated release notes v2.11.0 labels Sep 29, 2023
@@ -977,6 +979,7 @@ active | Integer | The number of active threads in the pool.
rejected | Integer | The number of tasks that have been rejected.
largest | Integer | The peak number of threads in the pool.
completed | Integer | The number of tasks completed.
total_wait_time | Integer | The total time tasks spent waiting in the thread pool queue.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not all thread_pools support this metric today -- only search, search_throttled, and index_searcher thread_pools support it. Not sure what the right place to mention this (if at all) is

@kolchfa-aws kolchfa-aws added the 4 - Doc review PR: Doc review in progress label Oct 3, 2023
Copy link
Contributor

@hdhalter hdhalter left a comment

Choose a reason for hiding this comment

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

Looks good! A couple nits.

_api-reference/nodes-apis/nodes-stats.md Outdated Show resolved Hide resolved
_api-reference/nodes-apis/nodes-stats.md Outdated Show resolved Hide resolved
kolchfa-aws and others added 2 commits October 3, 2023 11:54
Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@kolchfa-aws One change and one comment. Thanks!

_api-reference/nodes-apis/nodes-stats.md Outdated Show resolved Hide resolved
@@ -1066,7 +1069,7 @@ total.count | Integer | The total number of documents ingested by the node.
total.time_in_millis | Integer | The total amount of time for preprocessing ingest documents, in milliseconds.
total.current | Integer | The total number of documents that are currently being ingested by the node.
total.failed | Integer | The total number of failed ingestions for the node.
pipelines | Object | Ingest pipeline statistics for the node. Each pipeline is a nested object specified by its ID with the properties listed below.
pipelines | Object | Ingest pipeline statistics for the node. Each pipeline is a nested object specified by its ID with the following properties.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should "with" be "and has", as on line 1108?

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
@kolchfa-aws kolchfa-aws added 6 - Done but waiting to merge PR: The work is done and ready to merge and removed 4 - Doc review PR: Doc review in progress labels Oct 3, 2023
@@ -977,6 +979,7 @@ active | Integer | The number of active threads in the pool.
rejected | Integer | The number of tasks that have been rejected.
largest | Integer | The peak number of threads in the pool.
completed | Integer | The number of tasks completed.
total_wait_time | Integer | The total amount of time tasks spent waiting in the thread pool queue. Currently, only `search`, `search_throttled`, and `index_searcher` thread pools support this metric.
Copy link
Contributor

Choose a reason for hiding this comment

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

@kolchfa-aws @jed326 We are saying that this is supported for search threadpool but in the example above it is showing up for ad-batch-task-threadpool and OPENSEARCH_ML_TASK_THREAD_POOL. So either we add search threadpool stats in above example with this new metric or remove the new metric from those threadpool

@kolchfa-aws kolchfa-aws merged commit 1364059 into main Oct 16, 2023
4 checks passed
harshavamsi pushed a commit to harshavamsi/documentation-website that referenced this pull request Oct 31, 2023
…#5120)

* Add total wait time to thread pool in nodes stats

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Signed-off-by: kolchfa-aws <[email protected]>

* Removed total wait time from response

Signed-off-by: Fanit Kolchina <[email protected]>

---------

Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Co-authored-by: Heather Halter <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
vagimeli pushed a commit that referenced this pull request Dec 21, 2023
* Add total wait time to thread pool in nodes stats

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Update _api-reference/nodes-apis/nodes-stats.md

Signed-off-by: kolchfa-aws <[email protected]>

* Removed total wait time from response

Signed-off-by: Fanit Kolchina <[email protected]>

---------

Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Co-authored-by: Heather Halter <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
@kolchfa-aws kolchfa-aws deleted the thread-wait branch March 28, 2024 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6 - Done but waiting to merge PR: The work is done and ready to merge release-notes PR: Include this PR in the automated release notes v2.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] New threadpool metric thread_pool.total_wait_time
5 participants