branch-as-2.10 validation #25
Closed
Conversation
This file contains 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
…19166) (cherry picked from commit 4d7c7d0) Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zixuan Liu <[email protected]>
… remote admin fails (apache#23028) (cherry picked from commit 88ebe78) Signed-off-by: Zixuan Liu <[email protected]>
) (cherry picked from commit 5869791) Signed-off-by: Zixuan Liu <[email protected]>
* feat:return true if entry exists in cache * feat:return true if entry exists in cache * chore: add comment Co-authored-by: zhiyuanlei <[email protected]> Master Issue: apache#16979 ### Motivation PR apache#12258 made changes to OpAddEntry which causes a memory leak when the entry is already in the cache. ### Modifications Modifications: Revert PR apache#12258 changes to OpAddEntry ### Documentation - [x] `doc-not-needed` (cherry picked from commit 374b3a1)
…pache#17105) Fixes apache#16979 ### Motivation apache#12258 introduced caching for backlogged consumers. When caching the entry, it is important to duplicate the `ByteBuffer` so that the reader index is not shared. The current code has a race condition where the `ByteBuffer` reference in the cache is shared with the dispatcher. When another consumer reads from the cache, the cache calls `duplicate()` on the shared `ByteBuffer`, which copies the current reader index, which might not be 0 if the original dispatcher read data from the `ByteBuffer`. Note: it seems like the caching `insert` method creates (or recycles) more `EntryImpl` instances than is really necessary. Changing that is outside this PR's scope, so I am going to leave it as is. ### Modifications * Create a new `Entry` before inserting it into the cache. * Add a new test to the `EntryCacheTest`. The test fails before this change and passes after it. * Update the `EntryCacheTest` mocking so that it returns unique entries when mocking reads from the bookkeeper. Before, all returned `LedgerEntry` objects had ledgerId 0 and entryId 0, which messed with the caching for the new test. ### Verifying this change This change includes a test that failed before the PR and passes after it. ### Documentation - [x] `doc-not-needed` (cherry picked from commit 76f4195)
…#17241) (cherry picked from commit 3a3a993) Signed-off-by: Zixuan Liu <[email protected]>
…m storage/cache to the write to the consumer channel) (apache#18245) * InflightReadsLimiter - limit the memory used by reads end-to-end (from storage/cache to the write to the consumer channel) Motivation: Broker can go out of memory due to many reads enqueued on the PersistentDispatcherMultipleConsumers dispatchMessagesThread (that is used in case of dispatcherDispatchMessagesInSubscriptionThread set to true, that is the default value) The limit of the amount of memory retained due to reads MUST take into account also the entries coming from the Cache. When dispatcherDispatchMessagesInSubscriptionThread is false (the behaviour of Pulsar 2.10) there is some kind of natural (but still unpredictable!!) back pressure mechanism because the thread that receives the entries from BK of the cache dispatches immediately and synchronously the entries to the consumer and releases them Modifications: - Add a new component (InflightReadsLimiter) that keeps track of the overall amount of memory retained due to inflight reads. - Add a new configuration entry managedLedgerMaxReadsInFlightSizeInMB - The feature is disabled by default - Add new metrics to track the values * Change error message * checkstyle * Fix license * remove duplicate method after cherry-pick * Rename onDeallocate (cherry picked from commit 6fec66b) Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zixuan Liu <[email protected]>
…pache#16968) (cherry picked from commit a88d952) Signed-off-by: Zixuan Liu <[email protected]>
…in PersistentDispatcherMultipleConsumers class (apache#17018) (cherry picked from commit abff91f) Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zixuan Liu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #xyz
Main Issue: #xyz
PIP: #xyz
Motivation
Modifications
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: