Skip to content

[improve][ml] RangeCache refactoring: test race conditions and prevent endless loops #22814

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.

Sign up for GitHub

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

Jump to bottom
Merged
lhotari merged 8 commits into from
May 31, 2024
Merged

[improve][ml] RangeCache refactoring: test race conditions and prevent endless loops #22814

lhotari merged 8 commits into from
May 31, 2024

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented May 31, 2024

Motivation

Follow up on #22789. Test a few basic race conditions.
The solution in #22789 could lead to an endless loop. This PR fixes this problem.

Race conditions are logged with info level, for example:

2024-05-31T08:57:44,823 - INFO  - [pool-2-thread-4:RangeCache@308] - Value was already released for key 59, removing entry without releasing the value
2024-05-31T08:57:44,824 - INFO  - [pool-2-thread-4:RangeCache@308] - Value was already released for key 153, removing entry without releasing the value
2024-05-31T08:57:44,824 - INFO  - [pool-2-thread-7:RangeCache@308] - Value was already released for key 153, removing entry without releasing the value
2024-05-31T08:57:44,824 - INFO  - [pool-2-thread-8:RangeCache@308] - Value was already released for key 60, removing entry without releasing the value
2024-05-31T08:57:44,827 - INFO  - [pool-2-thread-2:RangeCache@296] - Key 580 does not match the entry's value wrapper's key 660, removing entry by key without releasing the value
2024-05-31T08:57:44,827 - INFO  - [pool-2-thread-3:RangeCache@296] - Key 226 does not match the entry's value wrapper's key 668, removing entry by key without releasing the value
2024-05-31T08:57:44,827 - INFO  - [pool-2-thread-4:RangeCache@308] - Value was already released for key 741, removing entry without releasing the value
2024-05-31T08:57:44,829 - INFO  - [pool-2-thread-6:RangeCache@308] - Value was already released for key 980, removing entry without releasing the value
2024-05-31T08:57:44,830 - INFO  - [pool-2-thread-6:RangeCache@308] - Value was already released for key 1074, removing entry without releasing the value
2024-05-31T08:57:44,830 - INFO  - [pool-2-thread-3:RangeCache@296] - Key 1088 does not match the entry's value wrapper's key 1358, removing entry by key without releasing the value
2024-05-31T08:57:44,911 - INFO  - [main:RangeCache@337] - Value RangeCacheTest.RefString(s=1, matchingKey=123) does not match the key 1, removed entry without releasing the value
2024-05-31T08:57:44,912 - INFO  - [main:RangeCache@308] - Value was already released for key 1, removing entry without releasing the value

Modifications

  • test some invalid entry cases which could be a result of race conditions
  • prevent endless loops in evictLeastAccessedEntries, evictLEntriesBeforeTimestamp and clear.
    • invalid entries cannot be skipped since .firstEntry() method is used for iterating in removal. If the entry isn't removed and persists, it causes the endless loop.
  • add logic for skipping invalid entries for removeRange

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari requested a review from merlimat May 31, 2024 06:39
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 31, 2024
@lhotari lhotari force-pushed the lh-rangecache-followup branch from 7309311 to 28cf786 Compare May 31, 2024 06:44
@lhotari lhotari force-pushed the lh-rangecache-followup branch from 28cf786 to 9ca1777 Compare May 31, 2024 06:58
@lhotari
Ensure consistency of IdentityWrapper.getValue
6dd1db1 
- prevent getting the value of a different key if the wrapper has been recycled
This was referenced May 31, 2024
Draft
Merged
@codecov-commenter
Copy link

codecov-commenter commented May 31, 2024 •
edited
Loading

Codecov Report

❌ Patch coverage is 78.12500% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.25%. Comparing base (bbc6224) to head (bb1689c).
⚠️ Report is 1269 commits behind head on master.

Files with missing lines Patch % Lines
...org/apache/bookkeeper/mledger/util/RangeCache.java 78.12% 6 Missing and 8 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22814      +/-   ##
============================================
- Coverage     73.57%   73.25%   -0.33%     
+ Complexity    32624     2511   -30113     
============================================
  Files          1877     1889      +12     
  Lines        139502   141692    +2190     
  Branches      15299    15553     +254     
============================================
+ Hits         102638   103792    +1154     
- Misses        28908    29877     +969     
- Partials       7956     8023      +67
Flag Coverage Δ
inttests 27.36% <62.50%> (+2.77%) ⬆️
systests 24.76% <53.12%> (+0.44%) ⬆️
unittests 72.28% <78.12%> (-0.57%) ⬇️

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

Files with missing lines Coverage Δ
...org/apache/bookkeeper/mledger/util/RangeCache.java 85.36% <78.12%> (-10.04%) ⬇️

... and 389 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lhotari lhotari merged commit e731674 into apache:master May 31, 2024
@lhotari lhotari mentioned this pull request Jun 2, 2024
Merged
4 tasks
lhotari added a commit that referenced this pull request Jun 3, 2024
@lhotari
[improve][ml] RangeCache refactoring: test race conditions and preven…
99aed3a 
…t endless loops (#22814)

(cherry picked from commit e731674)
lhotari added a commit that referenced this pull request Jun 3, 2024
@lhotari
[improve][ml] RangeCache refactoring: test race conditions and preven…
1f04038 
…t endless loops (#22814)

(cherry picked from commit e731674)
lhotari added a commit that referenced this pull request Jun 3, 2024
@lhotari
[improve][ml] RangeCache refactoring: test race conditions and preven…
1c9e1e6 
…t endless loops (#22814)

(cherry picked from commit e731674)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 7, 2024
@lhotari @nikhil-ctds
[improve][ml] RangeCache refactoring: test race conditions and preven…
2fb1aec 
…t endless loops (apache#22814)

(cherry picked from commit e731674)
(cherry picked from commit 1f04038)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 7, 2024
@lhotari @srinath-ctds
[improve][ml] RangeCache refactoring: test race conditions and preven…
040465d 
…t endless loops (apache#22814)

(cherry picked from commit e731674)
(cherry picked from commit 1f04038)
@lhotari lhotari mentioned this pull request Jun 14, 2024
Open
3 tasks
@lhotari lhotari added this to the 4.0.0 milestone Oct 14, 2024
@lhotari lhotari mentioned this pull request Jan 28, 2025
Merged
4 tasks
@lhotari lhotari mentioned this pull request Feb 10, 2025
Merged
4 tasks
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
@lhotari @hanmz
[improve][ml] RangeCache refactoring: test race conditions and preven…
d14f360 
…t endless loops (apache#22814)
@lhotari lhotari mentioned this pull request May 31, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merlimat merlimat merlimat approved these changes

Assignees

No one assigned

Labels

cherry-picked/branch-3.0 cherry-picked/branch-3.2 cherry-picked/branch-3.3 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.6 release/3.2.4 release/3.3.1

Projects

None yet

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@lhotari @codecov-commenter @merlimat