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

[fix][broker] Fixed error when delayed messages trackers state grows to >1.5GB #16490

Merged
merged 9 commits into from
Jul 12, 2022

Conversation

merlimat
Copy link
Contributor

@merlimat merlimat commented Jul 9, 2022

Motivation

The delayed messages tracker is using a ByteBuf in direct memory to store the priority queue. When the number of messages tracked grows a lot, the array gets doubled up to 1.5 GB. The next size increase will fail because 3.0 GB would be bigger than the max size for ByteBuf, 2GB, since they're indexed by integers.

Modification

Instead of using a single ByteBuf, use a list of buffers with a max segment size.

The first buffer will be expanded as normal, though the other buffers will always be of fixed size.

This approach has multiple benefits:

  1. Allows the tracker for a single topic to not be limited by 2GB
  2. There is much less memory wasted, since we don't have to double the buffer each time, but rather just add more segments
  3. It will make expansions and shrinking of the tracker much more efficient since we will not have to copy the entire data set, just adding or removing segments.
  • doc-not-needed

@merlimat merlimat added type/bug The PR fixed a bug or issue reported a bug doc-not-needed Your PR changes do not impact docs release/2.10.2 release/2.9.4 labels Jul 9, 2022
@merlimat merlimat added this to the 2.11.0 milestone Jul 9, 2022
@merlimat merlimat self-assigned this Jul 9, 2022
}

public void increaseCapacity() {
if (capacity < MAX_SEGMENT_SIZE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The size of the last segment might also < MAX_SEGMENT_SIZE, we need to consider to update the capacity of the buffer?

Copy link
Contributor

Choose a reason for hiding this comment

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

And the last segment size < MAX_SEGMENT_SIZE will also affect the writeLong and readLong method because we always uses the MAX_SEGMENT_SIZE to calculate the index.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the segments, after the first one, will be created directly at MAX_SEGMENT_SIZE and will not get expanded/shrinked, only added or removed.

Copy link
Contributor

@codelipenghui codelipenghui Jul 9, 2022

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's true! I need to fix the constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.
Would you please add a unit test for SegmentedLongArray?

@merlimat
Copy link
Contributor Author

Overall looks good to me.
Would you please add a unit test for SegmentedLongArray?

👍 Added

@codelipenghui codelipenghui changed the title Fixed error when delayed messages trackers state grows to >1.5GB [fix][broker] Fixed error when delayed messages trackers state grows to >1.5GB Jul 12, 2022
@merlimat merlimat merged commit 09f4333 into apache:master Jul 12, 2022
@merlimat merlimat deleted the fix-delay-tracker branch July 12, 2022 23:13
merlimat added a commit that referenced this pull request Jul 12, 2022
…to >1.5GB (#16490)

* Fixed error when delayed messages trackers state grows to >1.5GB

* Fixed spotbugs issues

* Fixed javadocs

* In the constructor, ensure all segments after the first one are of max size

* Use poll to figure out where the test is stuck

* Added SegmentedLongArray specific unit test

* Removed unused imports
merlimat added a commit that referenced this pull request Jul 12, 2022
…to >1.5GB (#16490)

* Fixed error when delayed messages trackers state grows to >1.5GB

* Fixed spotbugs issues

* Fixed javadocs

* In the constructor, ensure all segments after the first one are of max size

* Use poll to figure out where the test is stuck

* Added SegmentedLongArray specific unit test

* Removed unused imports
@merlimat merlimat added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Jul 12, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jul 13, 2022
…to >1.5GB (apache#16490)

* Fixed error when delayed messages trackers state grows to >1.5GB

* Fixed spotbugs issues

* Fixed javadocs

* In the constructor, ensure all segments after the first one are of max size

* Use poll to figure out where the test is stuck

* Added SegmentedLongArray specific unit test

* Removed unused imports

(cherry picked from commit eca6c4a)
wuxuanqicn pushed a commit to wuxuanqicn/pulsar that referenced this pull request Jul 14, 2022
…to >1.5GB (apache#16490)

* Fixed error when delayed messages trackers state grows to >1.5GB

* Fixed spotbugs issues

* Fixed javadocs

* In the constructor, ensure all segments after the first one are of max size

* Use poll to figure out where the test is stuck

* Added SegmentedLongArray specific unit test

* Removed unused imports
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.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.9.4 release/2.10.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants