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] Support shrink for ConcurrentSortedLongPairSet #15354

Merged
merged 4 commits into from
Apr 28, 2022

Conversation

Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented Apr 27, 2022

Motivation

Sometimes the messages sent to consumers are delayed over 100ms and we find that the CPU is wasted on :

"BookKeeperClientWorker-OrderedExecutor-4-0" #64 prio=5 os_prio=0 cpu=12748552.31ms elapsed=42037.79s tid=0x00007f7b4dc05720 nid=0xaf runnable  [0x00007f7a376f4000]
   java.lang.Thread.State: RUNNABLE
	at org.apache.pulsar.common.util.collections.ConcurrentLongPairSet.forEach(ConcurrentLongPairSet.java:157)
	at org.apache.pulsar.common.util.collections.ConcurrentSortedLongPairSet.items(ConcurrentSortedLongPairSet.java:136)
	at org.apache.pulsar.common.util.collections.ConcurrentSortedLongPairSet.items(ConcurrentSortedLongPairSet.java:128)
	at org.apache.pulsar.common.util.collections.ConcurrentSortedLongPairSet.items(ConcurrentSortedLongPairSet.java:113)
	at org.apache.pulsar.broker.service.persistent.MessageRedeliveryController.getMessagesToReplayNow(MessageRedeliveryController.java:109)
	at 

Then we find there are many empty datas in the map from dump :
image

ConcurrentSortedLongPairSet implements based on ConcurrentLongPairSet, and ConcurrentLongPairSet supports shrink, so support shrink for ConcurrentSortedLongPairSet to avoid iterator the empty data.

But the original removeIf has a bug if autoShrink=true, because when iterator the section, the internal table may change after remove, so result in some data not removed.

Reproduce Test:

LongPairSet set = new ConcurrentSortedLongPairSet(128, 2, true);
set.add(2, 2);
set.add(1, 3);
set.add(3, 1);
set.add(2, 1);
set.add(3, 2);
set.add(1, 2);
set.add(1, 1);
int removeItems = set.removeIf((ledgerId, entryId) -> {
    return ComparisonChain.start().compare(ledgerId, 1).compare(entryId, 3)
                    .result() <= 0;
 });
assertEquals(removeItems, 3);   // Will fail, removeItems = 2, (1,1) is not removed.

Modification

  • Fix removeIf and add related test.

Documentation

  • no-need-doc
    (Please explain why)

@Technoboy- Technoboy- self-assigned this Apr 27, 2022
@Technoboy- Technoboy- changed the title [improve][broker] Support shrink for ConcurrentSortedLongPairSet [improve][broker] Support shrink for ConcurrentSortedLongPairSet Apr 27, 2022
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@RobertIndie RobertIndie added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Apr 28, 2022
@RobertIndie RobertIndie added this to the 2.11.0 milestone Apr 28, 2022
Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

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

LGTM, left a trivial comment.

Copy link
Member

@shibd shibd left a comment

Choose a reason for hiding this comment

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

LGTM

@Technoboy- Technoboy- merged commit 24d4d76 into apache:master Apr 28, 2022
codelipenghui pushed a commit that referenced this pull request Apr 28, 2022
codelipenghui pushed a commit that referenced this pull request Apr 28, 2022
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Apr 28, 2022
codelipenghui pushed a commit that referenced this pull request Apr 29, 2022
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Apr 29, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 4, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 9, 2022
@Technoboy- Technoboy- deleted the fix-sorted-long-pair-set branch August 10, 2022 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.8.4 release/2.9.3 release/2.10.1 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants