[improve][broker] Fix ServiceUnitStateCompactionStrategy to cover fast-forward cursor behavior after compaction #20110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t-forward cursor behavior after compaction
|
@heesung-sn Please add the following content to your PR description and select a checkbox: |
github-actions
bot
added
doc-not-needed
...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java
Show resolved
Hide resolved
Demogorgon314
approved these changes
Apr 17, 2023
|
/pulsarbot run-failure-checks |
Codecov Report
@@ Coverage Diff @@
## master #20110 +/- ##
=============================================
+ Coverage 37.68% 72.93% +35.25%
- Complexity 12519 31915 +19396
=============================================
Files 1691 1868 +177
Lines 128779 138341 +9562
Branches 14044 15220 +1176
=============================================
+ Hits 48529 100905 +52376
+ Misses 73936 29418 -44518
- Partials 6314 8018 +1704
Flags with carried forward coverage won't be shown. Click here to find out more.
|
gaoran10
reviewed
Apr 18, 2023
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
Show resolved
Hide resolved
gaoran10
approved these changes
Apr 18, 2023
|
/pulsarbot run-failure-checks |
RobertIndie
pushed a commit
that referenced
this pull request
Apr 18, 2023
…t-forward cursor behavior after compaction (#20110) Master Issue: #16691 ### Motivation Raising a PR to implement: #16691 After the compaction, the cursor can fast-forward to the compacted horizon when a large number of messages are compacted before the next read. Hence, ServiceUnitStateCompactionStrategy also needs to cover this case. Currently, the existing and slow(their states are far behind) tableviews with ServiceUnitStateCompactionStrategy could not accept those compacted messages. In the load balance extension context, this means the ownership data could be inconsistent among brokers. ### Modifications This PR - fixes ServiceUnitStateCompactionStrategy to accept the state data if its version is bigger than the current version +1. - (minor fix) does not repeatedly update the replication_clusters in the policies when creating the system namespace. This update redundantly triggers ZK watchers when restarting brokers. - sets closeWithoutWaitingClientDisconnect=true, upon unload(following the same setting as the modular LM's) (cherry picked from commit 6cfa468)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Master Issue: #16691
Motivation
Raising a PR to implement: #16691
After the compaction, the cursor can fast-forward to the compacted horizon when a large number of messages are compacted before the next read. Hence, ServiceUnitStateCompactionStrategy also needs to cover this case. Currently, the existing and slow(their states are far behind) tableviews with ServiceUnitStateCompactionStrategy could not accept those compacted messages. In the load balance extension context, this means the ownership data could be inconsistent among brokers.
Modifications
This PR
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeWe will have separate PRs to update the Doc later.
Matching PR in forked repository
PR in forked repository: heesung-sn#46