Skip to content

[cleanup][broker] Remove configuration entries that no longer exist in the code - #26291

Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
apache:masterfrom
SEPURI-SAI-KRISHNA:cleanup-dead-config-entries
Open

[cleanup][broker] Remove configuration entries that no longer exist in the code#26291
SEPURI-SAI-KRISHNA wants to merge 1 commit into
apache:masterfrom
SEPURI-SAI-KRISHNA:cleanup-dead-config-entries

Conversation

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown

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.

disableBrokerInterceptors was deleted from ServiceConfiguration by PIP-293 (pip/pip-293.md:
"This disableBrokerInterceptors config is very confusing, so we decide to delete it."), but the
entry survived in conf/broker.conf and conf/standalone.conf, where it reads:

# Enable or disable the broker interceptor, which is only used for testing for now
disableBrokerInterceptors=true

Shipped as true, that reasonably suggests broker interceptors are disabled by default and have to be
switched on here — which has not been the case since PIP-293.
tests/integration/src/test/java/org/apache/pulsar/tests/integration/plugins/TestBrokerInterceptors.java
also still sets PULSAR_PREFIX_disableBrokerInterceptors=false, which is a no-op.

bookkeeperClientMinAvailableBookiesInIsolationGroups has no field in ServiceConfiguration and
no 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 its
comment describes — falling back to the secondary isolation group when the primary has too few
bookies — is implemented in IsolatedBookieEnsemblePlacementPolicy, but keyed on the requested
ensemble size rather than on a configurable minimum, so the setting has nothing to control.

ServiceConfigurationTest#testConfigFileDefaults already compares conf/broker.conf against
ServiceConfiguration, but only in one direction: it iterates the Java bean properties and checks the
file 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.conf
  • conf/standalone.conf
  • deployment/terraform-ansible/templates/broker.conf (the isolation-groups key only; it never
    carried 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 disableBrokerInterceptors are in pip/pip-293.md, which is the
historical proposal record and is intentionally left untouched.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

ServiceConfigurationTest passes; it is the existing guard that loads conf/broker.conf and checks it
against ServiceConfiguration.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant