[fix][broker] Duplicate LedgerOffloader creation when namespace/topic…

[shibd]

Motivation

#21590

When namespace/topic policies are updated, will call this method to try to apply new policies of offload.

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java

Lines 1373 to 1392 in eebd821

	public LedgerOffloader getManagedLedgerOffloader(NamespaceName namespaceName, OffloadPoliciesImpl offloadPolicies) {
	if (offloadPolicies == null) {
	return getDefaultOffloader();
	}
	return ledgerOffloaderMap.compute(namespaceName, (ns, offloader) -> {
	try {
	if (offloader != null && Objects.equals(offloader.getOffloadPolicies(), offloadPolicies)) {
	return offloader;
	} else {
	if (offloader != null) {
	offloader.close();
	}
	return createManagedLedgerOffloader(offloadPolicies);
	}
	} catch (PulsarServerException e) {
	LOG.error("create ledgerOffloader failed for namespace {}", namespaceName.toString(), e);
	return new NullLedgerOffloader();
	}
	});
	}

But, in this case(#21590), Never hit 1380 lines. offloader.getOffloadPolicies() and offloadPolicies are always unequal.

Reason:

1. In [Bug] Duplicate LedgerOffloader creation when namespace/topic any policy is updated #21590 reproduce step, set a useless config: fileSystemProfilePath.
2. This configuration is then ignored when converted to properties, then offloader.getOffloadPolicies() result not be included fileSystemProfilePath
3. offloadPolicies is built from the broker configuration, which contains this configuration.
4. So, the two are not equal

Note:
In v3.1 after. This pr: #20804 introduces a managedLedgerExtraConfigurations variable, which is also always unequal if the user does not set the value. (null vs empty map)

So after v3.1, In this case(#21590), this issue will appear regardless of whether you have fileSystemProfilePath set.

Modifications

• Changed toProperties method: Regardless of the user's configuration, we should all convert.
• The toProperties and create(Properties properties) methods should be complementary.
• Fix managedLedgerExtraConfigurations conversion issue.

Verifying this change

Documentation

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

Matching PR in forked repository

[codecov-commenter]

Codecov Report

Merging #21591 (652999e) into master (403faa4) will decrease coverage by 0.09%.
Report is 1 commits behind head on master.
The diff coverage is 90.47%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #21591      +/-   ##
============================================
- Coverage     73.35%   73.27%   -0.09%     
+ Complexity    32711    32685      -26     
============================================
  Files          1892     1892              
  Lines        140707   140663      -44     
  Branches      15483    15484       +1     
============================================
- Hits         103211   103066     -145     
- Misses        29394    29493      +99     
- Partials       8102     8104       +2     

Flag	Coverage Δ
inttests	24.19% <28.57%> (-0.06%)	⬇️
systests	24.68% <71.42%> (-0.38%)	⬇️
unittests	72.58% <90.47%> (-0.02%)	⬇️

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

Files	Coverage Δ
...lsar/common/policies/data/OffloadPoliciesImpl.java	85.66% <90.47%> (+2.02%)	⬆️

... and 78 files with indirect coverage changes