-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[cleanup][broker] PIP 45: Deprecate zookeeper settings managedLedgerMaxUnackedRangesToPersistInZooKeeper #15099
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update broker.conf
, too.
Fixed, thanks to you. |
@momo-jun a soft reminder: here is a PR w/ doc-required label, could u pls follow up? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In case this PR (for ZK free) is merged into 2.10, I drafted a related doc PR to catch up. |
conf/broker.conf
Outdated
# than this limit then broker will persist unacked ranges into bookkeeper to avoid additional data overhead into | ||
# zookeeper. | ||
# Deprecated: use managedLedgerMaxUnackedRangesToPersistInMetadataStore | ||
managedLedgerMaxUnackedRangesToPersistInZooKeeper=1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can change the default value of the deprecated to -1
and then check if users changed -1
to other values, we should apply this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/pulsarbot rerun-failure-checks |
…axUnackedRangesToPersistInZooKeeper (apache#15099)
…axUnackedRangesToPersistInZooKeeper (apache#15099)
Motivation
To deprecate the zookeeper settings. PIP 45 introduced a unified metadata store API for Pulsar,
so we should make the metadata-related configuration start with
metadataStore
, notzooKeeper
/ZK
.Modifications
managedLedgerMaxUnackedRangesToPersistInZooKeeper
tomanagedLedgerMaxUnackedRangesToPersistInMetadataStore
maxUnackedRangesToPersistInZk
tomaxUnackedRangesToPersistInMetadataStore
.Verifying this change
Does this pull request potentially affect one of the following parts:
Documentation
doc-required
(Your PR needs to update docs and you will update later)
Compatibility
Introduce
getManagedLedgerMaxUnackedRangesToPersistInMetadataStore
method to compatible with previous configurations.