[cleanup][broker] Remove configuration entries that no longer exist in the code - #26291
Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Open
[cleanup][broker] Remove configuration entries that no longer exist in the code#26291SEPURI-SAI-KRISHNA wants to merge 1 commit into
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Conversation
…n the code Assisted-by: Claude Code
This file contains hidden or 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
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.
Motivation
Two settings are still documented and shipped in the sample configuration files, but nothing in the
codebase reads them. Operators who set them get silence rather than an error, and one of them is
actively misleading.
disableBrokerInterceptorswas deleted fromServiceConfigurationby PIP-293 (pip/pip-293.md:"This
disableBrokerInterceptorsconfig is very confusing, so we decide to delete it."), but theentry survived in
conf/broker.confandconf/standalone.conf, where it reads:Shipped as
true, that reasonably suggests broker interceptors are disabled by default and have to beswitched on here — which has not been the case since PIP-293.
tests/integration/src/test/java/org/apache/pulsar/tests/integration/plugins/TestBrokerInterceptors.javaalso still sets
PULSAR_PREFIX_disableBrokerInterceptors=false, which is a no-op.bookkeeperClientMinAvailableBookiesInIsolationGroupshas no field inServiceConfigurationandno reader anywhere in the tree. It is not picked up by the BookKeeper passthrough either, which only
forwards
bookkeeper_-prefixed keys (BookKeeperClientFactoryImpl.java:165). The behaviour itscomment describes — falling back to the secondary isolation group when the primary has too few
bookies — is implemented in
IsolatedBookieEnsemblePlacementPolicy, but keyed on the requestedensemble size rather than on a configurable minimum, so the setting has nothing to control.
ServiceConfigurationTest#testConfigFileDefaultsalready comparesconf/broker.confagainstServiceConfiguration, but only in one direction: it iterates the Java bean properties and checks thefile agrees. A key present in the file but absent from Java is invisible to it, which is how both of
these survived.
Modifications
Removed both entries, together with their comment blocks, from:
conf/broker.confconf/standalone.confdeployment/terraform-ansible/templates/broker.conf(the isolation-groups key only; it nevercarried the interceptor one)
and dropped the no-op environment variable from
TestBrokerInterceptors.No code behaviour changes: neither key was read before this change.
The remaining references to
disableBrokerInterceptorsare inpip/pip-293.md, which is thehistorical proposal record and is intentionally left untouched.
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
ServiceConfigurationTestpasses; it is the existing guard that loadsconf/broker.confand checks itagainst
ServiceConfiguration.Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Both removed entries were already inert, so no effective default changes — the box is checked only
because the sample configuration files are edited.
Assisted-by: Claude Code