-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[improve][ml]Release idle offloaded read handle only the ref count is 0 #24381
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
[improve][ml]Release idle offloaded read handle only the ref count is 0 #24381
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work @poorbarcode
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24381 +/- ##
============================================
+ Coverage 73.57% 74.26% +0.68%
+ Complexity 32624 32241 -383
============================================
Files 1877 1867 -10
Lines 139502 145267 +5765
Branches 15299 16610 +1311
============================================
+ Hits 102638 107881 +5243
+ Misses 28908 28852 -56
- Partials 7956 8534 +578
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit 639521b)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit dcbe23f)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit 639521b)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit 639521b)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit 639521b)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit 639521b)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit dcbe23f)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit 639521b)
… 0 (apache#24381) (cherry picked from commit 21406b5) (cherry picked from commit dcbe23f)
… 0 (apache#24381) (cherry picked from commit 21406b5)
Motivation
#19783 introduced a feature: release idle offloaded ledger handle to release the direct memory occupied. But it left a potential issue, which has been discussed here, and it can happen this way
ledgerHandle.readAsync
touch()
, updatelastAccessTimestamp
executor.queue
executor
is very busy, leading to delayed executions ofledgerHandle.readAsync
ledgerHandle.readAsync
execute eventually, but the ledger handle has been closedSince users can modify the threshold of marking the ledger handle as
idle
by the configurationmanagedLedgerInactiveOffloadedLedgerEvictionTimeSeconds
, it may increase the probability of the issue happeningModifications
pendingRead
to record how many pending reading requests are in progress, Pulsar release ledger handle whenpendingRead
is0,
and has idled for enough timeDocumentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: x