[fix][broker] Skip loading broker interceptor when disableBrokerInterceptors is true

[nodece]

Motivation

When disableBrokerInterceptors is true, we should skip loading the broker interceptor to avoid initializing it.

This configuration is true by default, whether enabled or not, it will not affect the broker feature.

By the way, the web service and broker service should keep the same behavior:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java

Lines 216 to 224 in b7f7e04

	boolean brokerInterceptorEnabled =
	pulsarService.getBrokerInterceptor() != null && !config.isDisableBrokerInterceptors();
	if (brokerInterceptorEnabled) {
	ExceptionHandler handler = new ExceptionHandler();
	// Enable PreInterceptFilter only when interceptors are enabled
	filterHolders.add(
	new FilterHolder(new PreInterceptFilter(pulsarService.getBrokerInterceptor(), handler)));
	filterHolders.add(new FilterHolder(new ProcessHandlerFilter(pulsarService.getBrokerInterceptor())));
	}

Modifications

• Check whether disableBrokerInterceptors value, and then load the broker interceptor
• Update disableBrokerInterceptors description

Documentation

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

[nodece]

/pulsarbot rerun-failure-checks

[codecov-commenter]

Codecov Report

Merging #20422 (7051026) into master (aa3bfcd) will decrease coverage by 0.01%.
The diff coverage is 90.90%.

@@             Coverage Diff              @@
##             master   #20422      +/-   ##
============================================
- Coverage     72.90%   72.90%   -0.01%     
- Complexity    31864    31880      +16     
============================================
  Files          1864     1864              
  Lines        138416   138417       +1     
  Branches      15188    15189       +1     
============================================
- Hits         100919   100917       -2     
- Misses        29477    29489      +12     
+ Partials       8020     8011       -9     

Flag	Coverage Δ
inttests	24.18% <27.27%> (+0.01%)	⬆️
systests	24.96% <27.27%> (-0.10%)	⬇️
unittests	72.18% <90.90%> (+0.01%)	⬆️

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

Impacted Files	Coverage Δ
...n/java/org/apache/pulsar/broker/PulsarService.java	81.68% <83.33%> (-0.43%)	⬇️
...ookkeeper/mledger/impl/ManagedLedgerMBeanImpl.java	89.31% <100.00%> (ø)
...kkeeper/mledger/impl/cache/EntryCacheDisabled.java	70.83% <100.00%> (ø)
...keeper/mledger/impl/cache/RangeEntryCacheImpl.java	58.75% <100.00%> (ø)

... and 65 files with indirect coverage changes

[tisonkun]

@nodece what is the real-world affect without this fix? Will the broker crash or thow exception?

This can be semantically correct while if we have a real world problem, we may add a test case to prevent regressions.

[nodece]

@tisonkun

@nodece what is the real-world affect without this fix?

Without this patch, the broker always inits the broker interceptor and calls it.

Will the broker crash or thow exception?

The broker works fine.

This can be semantically correct while if we have a real world problem, we may add a test case to prevent regressions.

Perhaps I should add a test to check this call.