[fix][broker] Fix messages could not expire due to incorrect client clock

[315157973]

Fixes #21347

Motivation

When the client machine's clock is incorrect (eg: set to 1 year later) and the Broker does not set the AppendBrokerTimestampMetadataInterceptor, the Ledger will not be cleaned up.
Because if the Broker's timestamp is not set, the expiration check will be based on the client's publish time.

Modifications

Always set the Broker's timestamp

Verifying this change

• Make sure that the change passes the CI checks.

Documentation

• doc-not-needed

Matching PR in forked repository

PR in forked repository: 315157973#12

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.54%. Comparing base (bbc6224) to head (2f97734).
Report is 520 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #21835      +/-   ##
============================================
+ Coverage     73.57%   74.54%   +0.96%     
- Complexity    32624    34145    +1521     
============================================
  Files          1877     1920      +43     
  Lines        139502   144467    +4965     
  Branches      15299    15806     +507     
============================================
+ Hits         102638   107690    +5052     
+ Misses        28908    28519     -389     
- Partials       7956     8258     +302     

Flag	Coverage Δ
inttests	27.58% <0.00%> (+3.00%)	⬆️
systests	24.74% <0.00%> (+0.42%)	⬆️
unittests	73.88% <100.00%> (+1.04%)	⬆️

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

Files	Coverage Δ
...va/org/apache/pulsar/common/protocol/Commands.java	91.14% <100.00%> (+0.54%)	⬆️

... and 491 files with indirect coverage changes

[lhotari]

Please also fix another issue with the reuse of the brokerEntryMetadata.
Replace BrokerEntryMetadata brokerEntryMetadata = BROKER_ENTRY_METADATA.get(); with BrokerEntryMetadata brokerEntryMetadata = BROKER_ENTRY_METADATA.get().clear(); (append .clear()) to ensure that the brokerTimestamp doesn't get reused from the previous time the instance was used. Otherwise the timestamp would never get updated.

[lhotari]

Please also update the comment in org.apache.pulsar.common.protocol.Commands#getEntryTimestamp . I guess that now the BrokerEntryMetadata with the timestamp will always be added as long as broker entry metadata is enabled (any interceptor is enabled)?

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

Lines 1602 to 1615 in 92e0e47

	if (isBrokerEntryMetadataEnabled() || isBrokerPayloadProcessorEnabled()) {
	// init managedLedger interceptor
	Set<BrokerEntryMetadataInterceptor> interceptors = new HashSet<>();
	for (BrokerEntryMetadataInterceptor interceptor : brokerEntryMetadataInterceptors) {
	// add individual AppendOffsetMetadataInterceptor for each topic
	if (interceptor instanceof AppendIndexMetadataInterceptor) {
	interceptors.add(new AppendIndexMetadataInterceptor());
	} else {
	interceptors.add(interceptor);
	}
	}
	managedLedgerConfig.setManagedLedgerInterceptor(
	new ManagedLedgerInterceptorImpl(interceptors, brokerEntryPayloadProcessors));
	}

[codelipenghui]

The broker timestamp will be set in line:1702 if the AppendBrokerTimestampMetadataInterceptor is enabled. Is this PR wants to set the broker timestamp without AppendBrokerTimestampMetadataInterceptor enabled?

[315157973]

Unless we can ensure that the AppendBrokerTimestampMetadataInterceptor cannot be disabled, this bug may always occur

[codelipenghui]

Ok, I think we can either make the AppendBrokerTimestampMetadataInterceptor enabled by default or remove the AppendBrokerTimestampMetadataInterceptor to always apply the broker timestamp.

Without this change, the broker timestamp will be enabled without AppendBrokerTimestampMetadataInterceptor. Essentially, we delete the functionality of the AppendBrokerTimestampMetadataInterceptor.

[315157973]

I opened a discuss in dev thread https://lists.apache.org/thread/0vtjt436xvlg0fr4ogchkjdvjcp56mvz

[315157973]

As #21940 is merged, close this PR