Invalidate the read handle after all cursors consumed. - #11389
Merged
codelipenghui merged 4 commits intoJul 22, 2021
Conversation
Currently, the read ReadHandle only invalidate when removing the ledger from the ManagedLedger. If the ManagedLedger have many ledgers(the topic might retain infinite data), we will get oom on the direct memory since we are using 1MB read cache by default for the offloaded data ReadHandle. If all the cursors are consumed the data, the ReadHandle can be closed safety. And if a cursor reset to an earlier position to consume the historical data, the ReadHandle will be reopen again.
codelipenghui
added
release/2.7.4
release/2.8.1
type/bug
codelipenghui
requested review from
315157973,
gaoran10,
hangc0276,
merlimat,
sijie and
zymap
July 20, 2021 10:13
Technoboy-
added a commit
to Technoboy-/pulsar
that referenced
this pull request
Jul 20, 2021
…ors consumed in branch-2.7.
eolivelli
reviewed
Jul 20, 2021
| protected final MetaStore store; | ||
|
|
||
| private final ConcurrentLongHashMap<CompletableFuture<ReadHandle>> ledgerCache = new ConcurrentLongHashMap<>( | ||
| protected final ConcurrentLongHashMap<CompletableFuture<ReadHandle>> ledgerCache = new ConcurrentLongHashMap<>( |
Contributor
There was a problem hiding this comment.
we can make it "package protected" in order to reduce visibility
Contributor
Author
|
/pulsarbot run-failure-checks |
merlimat
approved these changes
Jul 22, 2021
Technoboy-
pushed a commit
to Technoboy-/pulsar
that referenced
this pull request
Jul 22, 2021
### Motivation Currently, the read ReadHandle only invalidates when removing the ledger from the ManagedLedger. If the ManagedLedger has many ledgers(the topic might retain infinite data), we will get oom on the direct memory since we are using 1MB read cache by default for the offloaded data ReadHandle. If all the cursors are consumed data, the ReadHandle can be closed safely. And if a cursor reset to an earlier position to consume the historical data, the ReadHandle will be reopened again. ### Verifying this change New tests were added to ensure the ReadHandle had been invalidated property.
codelipenghui
added a commit
that referenced
this pull request
Jul 23, 2021
### Motivation Currently, the read ReadHandle only invalidates when removing the ledger from the ManagedLedger. If the ManagedLedger has many ledgers(the topic might retain infinite data), we will get oom on the direct memory since we are using 1MB read cache by default for the offloaded data ReadHandle. If all the cursors are consumed data, the ReadHandle can be closed safely. And if a cursor reset to an earlier position to consume the historical data, the ReadHandle will be reopened again. ### Verifying this change New tests were added to ensure the ReadHandle had been invalidated property. (cherry picked from commit 47deafc)
codelipenghui
added a commit
that referenced
this pull request
Jul 23, 2021
Currently, the read ReadHandle only invalidates when removing the ledger from the ManagedLedger. If the ManagedLedger has many ledgers(the topic might retain infinite data), we will get oom on the direct memory since we are using 1MB read cache by default for the offloaded data ReadHandle. If all the cursors are consumed data, the ReadHandle can be closed safely. And if a cursor reset to an earlier position to consume the historical data, the ReadHandle will be reopened again. New tests were added to ensure the ReadHandle had been invalidated property. (cherry picked from commit 47deafc)
bharanic-dev
pushed a commit
to bharanic-dev/pulsar
that referenced
this pull request
Mar 18, 2022
### Motivation Currently, the read ReadHandle only invalidates when removing the ledger from the ManagedLedger. If the ManagedLedger has many ledgers(the topic might retain infinite data), we will get oom on the direct memory since we are using 1MB read cache by default for the offloaded data ReadHandle. If all the cursors are consumed data, the ReadHandle can be closed safely. And if a cursor reset to an earlier position to consume the historical data, the ReadHandle will be reopened again. ### Verifying this change New tests were added to ensure the ReadHandle had been invalidated property.
4 tasks
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
4 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.
Motivation
Currently, the read ReadHandle only invalidates when removing the ledger from the ManagedLedger.
If the ManagedLedger has many ledgers(the topic might retain infinite data), we will get oom
on the direct memory since we are using 1MB read cache by default for the offloaded data ReadHandle.
If all the cursors are consumed data, the ReadHandle can be closed safely. And if a cursor reset
to an earlier position to consume the historical data, the ReadHandle will be reopened again.
Verifying this change
New tests were added to ensure the ReadHandle had been invalidated property.
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changes