Skip to content
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][broker] Use shrink map for trackerCache #19534

Merged
merged 3 commits into from
Feb 16, 2023

Conversation

wolfstudy
Copy link
Member

@wolfstudy wolfstudy commented Feb 16, 2023

Motivation

Currently, the trackerCache uses a ConcurrentLongLongPairHashMap that does not shrink, causing trackerCache to occupy a large amount of memory and has no way to release it.

By monitoring the market through the JVM, we can see that during that time period, Young GC and Full GC have been frequently triggered, resulting in a large delay in Young GC and Full GC, which in turn affects production and consumption.

image
image

But what is interesting is that we can see that after the Young GC and Full GC are triggered many times, the memory in the heap is not reclaimed, so the Young GC and Full GC are triggered frequently.

image

Therefore, based on the above background, we made a memory dump of the Broker node and found that the set of trackerCache occupies most of the memory and this part of memory uses a non-shrink map, which will never be released. For details, refer to the following dump information:

image

Modifications

  • Use shrink map for trackerCache

  • doc

  • doc-required

  • doc-not-needed

  • doc-complete

@github-actions
Copy link

@wolfstudy Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Feb 16, 2023
@wolfstudy wolfstudy added this to the 2.11.0 milestone Feb 16, 2023
Signed-off-by: xiaolongran <[email protected]>

public class InMemoryRedeliveryTracker implements RedeliveryTracker {

private ConcurrentLongLongPairHashMap trackerCache = new ConcurrentLongLongPairHashMap(256, 1);
private ConcurrentLongLongPairHashMap trackerCache = ConcurrentLongLongPairHashMap.newBuilder()
.concurrencyLevel(2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to change the 256,1 to 128,2 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed done, PTAL again

Signed-off-by: xiaolongran <[email protected]>
@lhotari
Copy link
Member

lhotari commented Feb 16, 2023

/pulsarbot rerun-failure-checks

@lhotari lhotari merged commit c0f89dc into apache:master Feb 16, 2023
liangyepianzhou pushed a commit that referenced this pull request Feb 25, 2023
Signed-off-by: xiaolongran <[email protected]>
(cherry picked from commit c0f89dc)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Feb 28, 2023
Signed-off-by: xiaolongran <[email protected]>
(cherry picked from commit c0f89dc)
(cherry picked from commit 0f455e6)
coderzc pushed a commit that referenced this pull request Mar 1, 2023
Signed-off-by: xiaolongran <[email protected]>
(cherry picked from commit c0f89dc)
@coderzc coderzc added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Mar 1, 2023
Technoboy- pushed a commit that referenced this pull request Mar 6, 2023
Annavar-satish pushed a commit to pandio-com/pulsar that referenced this pull request Mar 6, 2023
Signed-off-by: xiaolongran <[email protected]>
(cherry picked from commit c0f89dc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants