Optimize the memory usage of Cache Eviction #12045

315157973 · 2021-09-15T08:17:18Z

Motivation

Cache Eviction is executed every 100ms by default, and the frequency is very high. I have observed that it produces a lot of garbage

The optimized memory is as follow:

Modifications

1）When there is no cache, no longer check
2）Avoid traversing all cursors

Documentation

no-need-doc

lhotari · 2021-09-15T09:13:36Z

There's a related problem report about the garbage in #9958 .

lhotari

Great work @315157973 !

lhotari · 2021-09-15T09:19:16Z

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

-        }
-
-        return smallest;
+        return (PositionImpl) (activeCursors.getSlowestReader()).getReadPosition();


there's also a method called ManagedCursorContainer.getSlowestReaderPosition(), could that have been used?

it looks that is uses getMarkDeletedPosition() so it's probably different than activeCursors.getSlowestReader().getReadPosition()?

You are right, now it seems to ignore the non-durable cursor

code changes, I'm unsure about the solution.

315157973 · 2021-09-16T03:49:18Z

@codelipenghui @hangc0276 PTAL

315157973 · 2021-09-16T04:08:20Z

profile-0.jfr.zip
profile.jfr.zip

Here is the JFR record

managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainerTest.java

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainer.java

315157973 · 2021-09-22T09:57:27Z

jfr of the latest code
profile-78.jfr.zip

codelipenghui · 2021-09-23T01:50:04Z

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainer.java

@@ -69,18 +67,19 @@

    private final StampedLock rwLock = new StampedLock();

+    // Used to keep track of the slowest non-durable cursor.
+    private final ArrayList<Item> heapForNonDurableCursors = Lists.newArrayList();


Looks if we want the track the slowest position for the non-durable cursor, it's more easier to maintain durableCursorContainer and nonDurableCursorContainer in the ManagedLedger? so that we don't need to change the implementation of the ManageCursorContainer

315157973 · 2021-09-24T12:09:31Z

@codelipenghui PTAL

codelipenghui · 2021-09-24T13:02:42Z

@merlimat Please help review the PR, thanks.

eolivelli

great

hangc0276

great job!

315157973 · 2021-09-30T09:35:38Z

Is it ready to merge ?

(cherry picked from commit 0c22e0f)

…uest !72) Squash merge branch 'OptimizeMemoryCache' into '2.8.1' Fixes #<xyz> ### Motivation 优化cache内存使用

…on of active cursors when cacheEvictionByMarkDeletedPosition=false Fixes apache#16054 - calculate the sorted list of when a read position gets updated - this resolves apache#9958 in a proper way - apache#12045 broke the caching solution as explained in apache#16054

… active cursors when cacheEvictionByMarkDeletedPosition=false Fixes apache#16054 - calculate the sorted list of when a read position gets updated - this resolves apache#9958 in a proper way - apache#12045 broke the caching solution as explained in apache#16054 - remove invalid tests - fix tests - add more tests to handle corner cases

…sors (#17273) * [fix][broker] Fix broken build caused by conflict between #17195 and #16605 - #17195 changed the method signature that #16605 depended upon * [fix][broker] Keep sorted list of cursors ordered by read position of active cursors when cacheEvictionByMarkDeletedPosition=false Fixes #16054 - calculate the sorted list of when a read position gets updated - this resolves #9958 in a proper way - #12045 broke the caching solution as explained in #16054 - remove invalid tests - fix tests - add more tests to handle corner cases * Address review comment * Handle durable & non-durable in the correct way * Fix cache tests since now entries get evicted reactively * Address review comment about method names * Change signature for add method so that position must be passed - this is more consistent with cursorUpdated method where the position is passed * Update javadoc for ManagedCursorContainer * Address review comment * Simplify ManagedCursorContainer * Clarify javadoc * Ensure that cursors are tracked by making sure that initial position isn't null unintentionally * Prevent race in updating activeCursors

…sors (#17273) * [fix][broker] Fix broken build caused by conflict between #17195 and #16605 - #17195 changed the method signature that #16605 depended upon * [fix][broker] Keep sorted list of cursors ordered by read position of active cursors when cacheEvictionByMarkDeletedPosition=false Fixes #16054 - calculate the sorted list of when a read position gets updated - this resolves #9958 in a proper way - #12045 broke the caching solution as explained in #16054 - remove invalid tests - fix tests - add more tests to handle corner cases * Address review comment * Handle durable & non-durable in the correct way * Fix cache tests since now entries get evicted reactively * Address review comment about method names * Change signature for add method so that position must be passed - this is more consistent with cursorUpdated method where the position is passed * Update javadoc for ManagedCursorContainer * Address review comment * Simplify ManagedCursorContainer * Clarify javadoc * Ensure that cursors are tracked by making sure that initial position isn't null unintentionally * Prevent race in updating activeCursors (cherry picked from commit 856ef15)

Optimize the memory usage of Cache Eviction

a81be5c

315157973 requested review from merlimat, lhotari, hangc0276, eolivelli, codelipenghui and congbobo184 September 15, 2021 08:17

315157973 self-assigned this Sep 15, 2021

lhotari previously approved these changes Sep 15, 2021

View reviewed changes

lhotari reviewed Sep 15, 2021

View reviewed changes

add slowestReadPositionForActiveCursors

c05610b

lhotari self-requested a review September 15, 2021 15:45

codelipenghui reviewed Sep 18, 2021

View reviewed changes

managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainerTest.java Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainer.java Outdated Show resolved Hide resolved

codelipenghui added this to the 2.9.0 milestone Sep 18, 2021

codelipenghui added the release/2.8.2 label Sep 18, 2021

Fix can not move forward

0a42bb3

Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Sep 22, 2021

codelipenghui reviewed Sep 23, 2021

View reviewed changes

315157973 added 3 commits September 23, 2021 17:35

use nonDurableActiveCursorsContainer

bd47bb5

fix unit test

46860c8

fix unit test

95934f2

codelipenghui approved these changes Sep 24, 2021

View reviewed changes

eolivelli approved these changes Sep 24, 2021

View reviewed changes

hangc0276 approved these changes Sep 24, 2021

View reviewed changes

hangc0276 added area/broker release/2.7.4 labels Sep 24, 2021

eolivelli merged commit 0c22e0f into apache:master Sep 30, 2021

codelipenghui pushed a commit that referenced this pull request Oct 6, 2021

Optimize the memory usage of Cache Eviction (#12045)

44aaa47

(cherry picked from commit 0c22e0f)

codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Oct 6, 2021

eolivelli added the release/2.9.0 label Oct 6, 2021

codelipenghui pushed a commit that referenced this pull request Dec 11, 2021

Optimize the memory usage of Cache Eviction (#12045)

d846b43

(cherry picked from commit 0c22e0f)

codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Dec 11, 2021

lhotari mentioned this pull request Jan 28, 2022

Excessive CPU use in idle mode #14015

Closed

bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022

Optimize the memory usage of Cache Eviction (apache#12045)

28f877c

This was referenced Jun 14, 2022

[ML] Fix thread safety issues in accessing ManagedCursorContainer.heap ArrayList #16049

Merged

Broker cache eviction evicts entries that haven't been read by active consumers #16054

Closed

sijie mentioned this pull request Jun 14, 2022

ISSUE-16054: Broker cache eviction evicts entries that haven't been read by active consumers streamnative/pulsar-archived#4375

Open

lhotari mentioned this pull request Aug 25, 2022

[fix][broker] Fix broker cache eviction of entries read by active cursors #17273

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize the memory usage of Cache Eviction #12045

Optimize the memory usage of Cache Eviction #12045

315157973 commented Sep 15, 2021

lhotari commented Sep 15, 2021

lhotari left a comment

lhotari Sep 15, 2021

lhotari Sep 15, 2021

315157973 Sep 15, 2021

315157973 commented Sep 16, 2021

315157973 commented Sep 16, 2021 •

edited

Loading

315157973 commented Sep 22, 2021

codelipenghui Sep 23, 2021

315157973 Sep 23, 2021

315157973 commented Sep 24, 2021

codelipenghui commented Sep 24, 2021

eolivelli left a comment

hangc0276 left a comment

315157973 commented Sep 30, 2021

Optimize the memory usage of Cache Eviction #12045

Optimize the memory usage of Cache Eviction #12045

Conversation

315157973 commented Sep 15, 2021

Motivation

Modifications

Documentation

lhotari commented Sep 15, 2021

lhotari left a comment

Choose a reason for hiding this comment

lhotari Sep 15, 2021

Choose a reason for hiding this comment

lhotari Sep 15, 2021

Choose a reason for hiding this comment

315157973 Sep 15, 2021

Choose a reason for hiding this comment

315157973 commented Sep 16, 2021

315157973 commented Sep 16, 2021 • edited Loading

315157973 commented Sep 22, 2021

codelipenghui Sep 23, 2021

Choose a reason for hiding this comment

315157973 Sep 23, 2021

Choose a reason for hiding this comment

315157973 commented Sep 24, 2021

codelipenghui commented Sep 24, 2021

eolivelli left a comment

Choose a reason for hiding this comment

hangc0276 left a comment

Choose a reason for hiding this comment

315157973 commented Sep 30, 2021

315157973 commented Sep 16, 2021 •

edited

Loading