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

[Tiered Caching] [Bug Fix] Use concurrentMap instead of HashMap to fix Concurrent Modification Exception #14134

Closed

Conversation

kiranprakash154
Copy link
Contributor

@kiranprakash154 kiranprakash154 commented Jun 10, 2024

Description

#14032 reported msearches failing with a Concurrent Modification Exception, though they were not able to repro it consistently they were able to attach a debugger and find out the line throwing the exception, which was

cleanupKeyToCountMap.computeIfAbsent(shardId, k -> new HashMap<>()).merge(cleanupKey.readerCacheKeyId, 1, Integer::sum);

We are using a HashMap (not thread safe) for the inner map of the cleanupKeyToCountMap and hence it throws a Concurrent Modification Exception when the map is getting updated by multiple threads concurrently.

The fix is to use a thread safe Concurrent Map instead.

Related Issues

Resolves #14032

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.

@kiranprakash154 kiranprakash154 self-assigned this Jun 10, 2024
@kiranprakash154 kiranprakash154 added the backport 2.x Backport to 2.x branch label Jun 10, 2024
Signed-off-by: Kiran Prakash <[email protected]>
@kiranprakash154 kiranprakash154 changed the title use concurrentMap instead of Hashmap [Tiered Caching] [Bug Fix] Use concurrentMap instead of Hashmap to fix Concurrent Modification Exception Jun 10, 2024
@kiranprakash154 kiranprakash154 changed the title [Tiered Caching] [Bug Fix] Use concurrentMap instead of Hashmap to fix Concurrent Modification Exception [Tiered Caching] [Bug Fix] Use concurrentMap instead of HashMap to fix Concurrent Modification Exception Jun 10, 2024
Copy link
Contributor

❌ Gradle check result for 58aa93a: 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 aabf139: 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 39642ab: 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?

@github-actions github-actions bot added bug Something isn't working Search Search query, autocomplete ...etc v2.15.0 Issues and PRs related to version 2.15.0 labels Jun 12, 2024
Copy link
Contributor

✅ Gradle check result for cc8b6b7: SUCCESS

Copy link

codecov bot commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.70%. Comparing base (b15cb0c) to head (cc8b6b7).
Report is 413 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #14134      +/-   ##
============================================
+ Coverage     71.42%   71.70%   +0.28%     
- Complexity    59978    62051    +2073     
============================================
  Files          4985     5117     +132     
  Lines        282275   291687    +9412     
  Branches      40946    42166    +1220     
============================================
+ Hits         201603   209156    +7553     
- Misses        63999    65278    +1279     
- Partials      16673    17253     +580     

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

Copy link
Member

@dblock dblock 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, or at least harmless, ready to merge?

@kiranprakash154
Copy link
Contributor Author

Looks good, or at least harmless, ready to merge?

Sorry @dblock , closed this since i have a duplicate PR open - #14221

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch bug Something isn't working Search:Performance Search Search query, autocomplete ...etc v2.15.0 Issues and PRs related to version 2.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] sporadic concurrent_modification_exception during query in 2.14
2 participants