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

[Backport 2.x] Core and Plugin changes for query-level resource usages tracking #14085

Merged
merged 4 commits into from
Jun 10, 2024

Conversation

ansjcy
Copy link
Member

@ansjcy ansjcy commented Jun 8, 2024

Description

This is a manual backport PR for Query-level resource usages tracking, and the query insights plugin support to consume the query level resource usage.

Related Issues

Original PRs:
#13172
#14084
#13739
#14147

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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

Copy link
Contributor

github-actions bot commented Jun 8, 2024

❌ Gradle check result for d3c82b6:

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@ansjcy ansjcy changed the title [Backport 2.x] Query-level resource usages tracking [Backport 2.x] Core and Plugin changes for query-level resource usages tracking Jun 8, 2024
Copy link
Contributor

github-actions bot commented Jun 8, 2024

❌ Gradle check result for a862c82:

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch 2 times, most recently from f213ca7 to 9cc51bc Compare June 10, 2024 05:59
Copy link
Contributor

❌ Gradle check result for f213ca7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 9cc51bc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch from 9cc51bc to 7b01702 Compare June 10, 2024 06:22
Copy link
Contributor

❌ Gradle check result for 7b01702: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch from 7b01702 to c56af99 Compare June 10, 2024 07:03
Copy link
Contributor

❌ Gradle check result for c56af99: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch from c56af99 to aeca941 Compare June 10, 2024 16:59
Copy link
Contributor

❕ Gradle check result for aeca941: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch 2 times, most recently from 5cbe7fe to e2df8ad Compare June 10, 2024 18:39
@jed326
Copy link
Collaborator

jed326 commented Jun 10, 2024

Breaking changes is going to fail until #14143 is merged

Copy link
Contributor

✅ Gradle check result for 5cbe7fe: SUCCESS

@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch from e2df8ad to 0bfb26e Compare June 10, 2024 19:09
Copy link
Contributor

✅ Gradle check result for e2df8ad: SUCCESS

* Query-level resource usages tracking

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* Moving TaskResourceTrackingService to clusterService

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* use shard response header to piggyback task resource usages

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* split changes for query insights plugin

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* improve the supplier logic and other misc items

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* track resource usage for failed requests

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* move resource usages interactions into TaskResourceTrackingService

Signed-off-by: Chenyang Ji <cyji@amazon.com>

---------

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit 3d1fa98)
…ct#14084)

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit c8f0b6d)
…h-project#13739)

* consume query level cpu and memory usage in query insights

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* handle failed requests metrics in query insights

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* refactor the code to make it more maintainable

Signed-off-by: Chenyang Ji <cyji@amazon.com>

---------

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit 04a417a)
Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit b403fdc)
@ansjcy ansjcy force-pushed the backport/backport-13172-to-2.x branch from 0bfb26e to da4367e Compare June 10, 2024 19:37
Copy link
Contributor

✅ Gradle check result for 0bfb26e: SUCCESS

Copy link

codecov bot commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 78.67647% with 58 lines in your changes missing coverage. Please review.

Project coverage is 71.40%. Comparing base (0dd892c) to head (da4367e).
Report is 338 commits behind head on 2.x.

Files Patch % Lines
.../opensearch/tasks/TaskResourceTrackingService.java 67.50% 8 Missing and 5 partials ⚠️
.../insights/core/listener/QueryInsightsListener.java 69.23% 10 Missing and 2 partials ⚠️
...in/insights/core/service/QueryInsightsService.java 50.00% 8 Missing and 4 partials ⚠️
...ansport/top_queries/TransportTopQueriesAction.java 20.00% 7 Missing and 1 partial ⚠️
...h/core/tasks/resourcetracker/TaskResourceInfo.java 93.05% 1 Missing and 4 partials ⚠️
...erver/src/main/java/org/opensearch/tasks/Task.java 57.14% 1 Missing and 2 partials ⚠️
...lugin/insights/core/service/TopQueriesService.java 0.00% 2 Missing ⚠️
...action/search/SearchRequestOperationsListener.java 75.00% 2 Missing ⚠️
...opensearch/action/search/SearchRequestContext.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x   #14085      +/-   ##
============================================
+ Coverage     71.28%   71.40%   +0.12%     
- Complexity    60145    61836    +1691     
============================================
  Files          4957     5071     +114     
  Lines        282799   290401    +7602     
  Branches      41409    42338     +929     
============================================
+ Hits         201591   207375    +5784     
- Misses        64189    65572    +1383     
- Partials      17019    17454     +435     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

✅ Gradle check result for da4367e: SUCCESS

Copy link
Collaborator

@jed326 jed326 left a comment

Choose a reason for hiding this comment

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

Waiting to merge #14147 first before this one.

@jed326 jed326 merged commit 0634ccd into opensearch-project:2.x Jun 10, 2024
32 checks passed
kkewwei pushed a commit to kkewwei/OpenSearch that referenced this pull request Jul 24, 2024
…s tracking (opensearch-project#14085)

* Query-level resource usages tracking (opensearch-project#13172)

* Query-level resource usages tracking

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* Moving TaskResourceTrackingService to clusterService

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* use shard response header to piggyback task resource usages

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* split changes for query insights plugin

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* improve the supplier logic and other misc items

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* track resource usage for failed requests

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* move resource usages interactions into TaskResourceTrackingService

Signed-off-by: Chenyang Ji <cyji@amazon.com>

---------

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit 3d1fa98)

* fix concurrent modification issue in thread context (opensearch-project#14084)

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit c8f0b6d)

* consume query level cpu and memory usage in query insights (opensearch-project#13739)

* consume query level cpu and memory usage in query insights

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* handle failed requests metrics in query insights

Signed-off-by: Chenyang Ji <cyji@amazon.com>

* refactor the code to make it more maintainable

Signed-off-by: Chenyang Ji <cyji@amazon.com>

---------

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit 04a417a)

* fix japicmp check for threadContext

Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit b403fdc)
Signed-off-by: kkewwei <kkewwei@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants