Avoid cancelltion checks on every advance() call for posting enums - #22856
Open
sgup432 wants to merge 2 commits into
Open
Avoid cancelltion checks on every advance() call for posting enums#22856sgup432 wants to merge 2 commits into
sgup432 wants to merge 2 commits into
Conversation
Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com>
sgup432
requested review from
a team,
Bukhtawar,
alchemist51,
andrross,
ashking94,
cwperks,
dbwiddis,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
owaiskazi19,
reta,
sachinpkale,
saratvemulapalli,
shwetathareja and
sohami
as code owners
August 26, 2026 20:08
Contributor
PR Reviewer Guide 🔍(Review updated until commit 44dc105)Here are some key observations to aid the review process:
|
jainankitk
approved these changes
Aug 26, 2026
Contributor
|
Persistent review updated to latest commit 44dc105 |
Contributor
|
❌ Gradle check result for 44dc105: 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? |
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22856 +/- ##
============================================
+ Coverage 71.53% 71.64% +0.11%
- Complexity 77249 77390 +141
============================================
Files 6170 6170
Lines 359710 359710
Branches 52460 52460
============================================
+ Hits 257318 257718 +400
+ Misses 81965 81589 -376
+ Partials 20427 20403 -24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@jainankitk Can we merge this in now? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
As part of earlier change(in OS 3.7, PR) done to strengthen the system so that we are able to stop the long running query if it was already cancelled, we ended up adding a check on advance() without sampling. In worst advance() might behave like nextDoc() where we end up iterating all the docs, so doing cancellation checks on every doc can cause high CPU and also unnecessary memory allocation.
This was visible for one of the OpenSearch user. I took a 5min JFR profile for their domain to root cause latency/CPU spikes after upgrade to 3.7.
CPU profile:
Memory allocation:
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
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.