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][PIP-195] Merge multiple buckets at once #19927

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

coderzc
Copy link
Member

@coderzc coderzc commented Mar 26, 2023

PIP: #16763

Motivation

Currently, we select two buckets at once to merge, which causes the merge operation to be frequently triggered. We should merge multiple buckets at once to avoid frequent merges.

Modifications

  • Use k-way merge to implement CombinedSegmentDelayedIndexQueue
  • Merge multiple buckets at once, the default select 4 buckets to merge. If the tracker can't find able merged buckets will try to reduce the number of merged buckets.
  • Improve clear code
  • Improve logs

Verifying this change

  • Make sure that the change passes the CI checks.

(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:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@coderzc coderzc self-assigned this Mar 26, 2023
@coderzc coderzc added this to the 3.0.0 milestone Mar 26, 2023
@coderzc coderzc added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Mar 26, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 26, 2023
@coderzc coderzc added area/broker ready-to-test and removed doc-not-needed Your PR changes do not impact docs labels Mar 26, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 26, 2023
@coderzc coderzc closed this Mar 26, 2023
@coderzc coderzc reopened this Mar 26, 2023
@coderzc coderzc force-pushed the improve/bucket_delayed_mege_num branch from bee2dba to 9162451 Compare March 26, 2023 06:12
@coderzc coderzc force-pushed the improve/bucket_delayed_mege_num branch from 9162451 to 4109066 Compare March 26, 2023 06:48
@codecov-commenter
Copy link

Codecov Report

Merging #19927 (4109066) into master (329e80b) will increase coverage by 0.44%.
The diff coverage is 0.84%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19927      +/-   ##
============================================
+ Coverage     61.50%   61.95%   +0.44%     
- Complexity    25755    26070     +315     
============================================
  Files          1859     1859              
  Lines        136792   136796       +4     
  Branches      15043    15042       -1     
============================================
+ Hits          84129    84747     +618     
+ Misses        44857    44247     -610     
+ Partials       7806     7802       -4     
Flag Coverage Δ
inttests 24.42% <0.84%> (-0.06%) ⬇️
systests 25.11% <0.00%> (+0.09%) ⬆️
unittests 59.27% <0.84%> (+0.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...r/delayed/bucket/BucketDelayedDeliveryTracker.java 0.00% <0.00%> (ø)
...layed/bucket/CombinedSegmentDelayedIndexQueue.java 0.00% <0.00%> (ø)
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 78.34% <100.00%> (+0.29%) ⬆️

... and 186 files with indirect coverage changes

@coderzc coderzc merged commit bbf5273 into apache:master Mar 29, 2023
@coderzc coderzc deleted the improve/bucket_delayed_mege_num branch March 29, 2023 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs ready-to-test 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.

3 participants