Visitar URL original
Fix the ConcurrentModificationException for And/Or DocIdSet by Jackie-Jiang · Pull Request #12611 · apache/pinot · GitHub
Skip to content

Fix the ConcurrentModificationException for And/Or DocIdSet - #12611

Merged
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:fix_thread_safety_doc_id_set
Mar 11, 2024
Merged

Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:fix_thread_safety_doc_id_set

Conversation

@Jackie-Jiang

Copy link
Copy Markdown
Contributor

Fix #12368

When a query is early terminated, some segments might still be executing while the combine operator is collecting execution stats. This can cause ConcurrentModificationException for AndDocIdSet and OrDocIdSet since both methods are accessing the same List, and one is modifying it (introduced in #11200).

This PR fixed issue by keeping the scan based DocIdSets in a separate list

@codecov-commenter

codecov-commenter commented Mar 8, 2024 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (59551e4) to head (b5acc23).
⚠️ Report is 3864 commits behind head on master.

Files with missing lines Patch % Lines
...che/pinot/core/operator/docidsets/AndDocIdSet.java 0.00% 22 Missing ⚠️
...ache/pinot/core/operator/docidsets/OrDocIdSet.java 0.00% 21 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (59551e4) and HEAD (b5acc23). Click for more details.

HEAD has 34 uploads less than BASE
Flag BASE (59551e4) HEAD (b5acc23)
integration 7 5
temurin 12 5
java-21 7 2
skip-bytebuffers-true 3 1
skip-bytebuffers-false 7 3
unittests 5 0
unittests1 2 0
java-11 5 3
unittests2 3 0
integration1 2 1
custom-integration1 2 1
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #12611       +/-   ##
=============================================
- Coverage     61.75%    0.00%   -61.75%     
+ Complexity      207        6      -201     
=============================================
  Files          2436     2375       -61     
  Lines        133233   130119     -3114     
  Branches      20636    20196      -440     
=============================================
- Hits          82274        6    -82268     
- Misses        44911   130113    +85202     
+ Partials       6048        0     -6048     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 <0.01% <0.00%> (-61.71%) ⬇️
java-21 0.00% <0.00%> (-61.63%) ⬇️
skip-bytebuffers-false <0.01% <0.00%> (-61.75%) ⬇️
skip-bytebuffers-true 0.00% <0.00%> (-27.73%) ⬇️
temurin <0.01% <0.00%> (-61.75%) ⬇️
unittests ?
unittests1 ?
unittests2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang
Jackie-Jiang merged commit 8b88697 into apache:master Mar 11, 2024
@Jackie-Jiang
Jackie-Jiang deleted the fix_thread_safety_doc_id_set branch March 11, 2024 17:45
@ankitsultana

Copy link
Copy Markdown
Contributor

@Jackie-Jiang : we are seeing this in our internal 1.1 release cut build as well. We didn't add this to that release?

cc: @vvivekiyer

@Jackie-Jiang

Copy link
Copy Markdown
Contributor Author

No, seems it is not included..

tarun11Mavani added a commit to tarun11Mavani/pinot that referenced this pull request May 9, 2025
…tanceof BitmapBasedDocIdIterator. Fixes bug introduced in apache#12611
tarun11Mavani added a commit to tarun11Mavani/pinot that referenced this pull request May 9, 2025
…tanceof BitmapBasedDocIdIterator. Fixes bug introduced in apache#12611
chenboat pushed a commit that referenced this pull request May 12, 2025
* Add docIdIterator to bitmapBasedDocIdIterators when docIdIterator instanceof BitmapBasedDocIdIterator. Fixes bug introduced in #12611

* Adding UTs

* fix checkstyle
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.

[Flaky test] MultiStageEngineCustomTenantIntegrationTest.testGeneratedQueries()

4 participants