-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
fix concurrent modification exceptions in thread context #14084
fix concurrent modification exceptions in thread context #14084
Conversation
server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java
Outdated
Show resolved
Hide resolved
LGTM apart from the minor comment. Will approve after the change |
666af8c
to
86c4909
Compare
86c4909
to
768da23
Compare
❕ Gradle check result for 666af8c: 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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14084 +/- ##
============================================
+ Coverage 71.42% 71.69% +0.27%
- Complexity 59978 61622 +1644
============================================
Files 4985 5082 +97
Lines 282275 289233 +6958
Branches 40946 41853 +907
============================================
+ Hits 201603 207360 +5757
- Misses 63999 64732 +733
- Partials 16673 17141 +468 ☔ View full report in Codecov by Sentry. |
❌ Gradle check result for 86c4909: 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? |
❌ Gradle check result for 768da23: 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? |
768da23
to
70f95c0
Compare
Signed-off-by: Chenyang Ji <cyji@amazon.com>
70f95c0
to
95bcf01
Compare
❌ Gradle check result for 70f95c0: 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? |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-14084-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c8f0b6da6def4b8e78cd17274e5a4271e844a71f
# Push it to GitHub
git push --set-upstream origin backport/backport-14084-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x Then, create a pull request where the |
…ct#14084) Signed-off-by: Chenyang Ji <cyji@amazon.com>
…s tracking (#14085) * Query-level resource usages tracking (#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 (#14084) Signed-off-by: Chenyang Ji <cyji@amazon.com> (cherry picked from commit c8f0b6d) * consume query level cpu and memory usage in query insights (#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)
…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>
…ct#14084) Signed-off-by: Chenyang Ji <cyji@amazon.com>
Description
Current implementation to inject headers in thread context caused concurrent modification errors since other threads can access the headers at the same time when we delete it.
This is caused by removing a key from a map while other threads are iterating the map at the same time.
This PR removes the delete logic and make replacing existing key logic self contained in put headers function.
Related Issues
Related to the above build failure
Check List
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.