[fix][broker] Ignore non-retryable errors when loading topic-level policies before loading topics

[BewareMyPower]

Motivation

#21445 introduces an implicit requirement that if the topic policies cannot be loaded, BrokerService#getTopic will fail and then the client will retry loading the topic.

It could break some existing usages like the tests in the C++ client: apache/pulsar-client-cpp#394

This change is applied only to avoid the race condition when unloading a namespace bundle. However, the client should not fail if the topic-level policies are not available.

Modifications

If the topic policies cannot be loaded due to a non-retryable error, we should not fail the getTopic. For retryable errors, the client will still retry until the broker gets the topic-level policies successfully after some attempts.

Modify TokenAuthenticatedProducerConsumerTest to protect the change.

Note: This PR also changes the isRetriableError semantics. To keep the compatibility, should we avoid this change?

Documentation

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

Matching PR in forked repository

PR in forked repository:

[codelipenghui]

This change is applied only to avoid the race condition when unloading a namespace bundle. However, the client should not fail if the topic-level policies are not available.

The risk is if users have data retention policy on the topic but no data retention on the namespace, the produced data will be removed by the broker if no subscriptions.

For example, you might have 10GB produced data with data retention policy before. If the topic been unloaded and reload by another broker without the topic policy(failed to load the topic policy), all the 10GB data will be removed permanently. It could be high risk to users.

And other policies will also have the similar problem, persistence policy (replicas), backlog policy, geo-replication …

[BewareMyPower]

For example, you might have 10GB produced data with data retention policy before. If the topic been unloaded and reload by another broker without the topic policy,

I don't think this issue is avoided. See

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

Line 1064 in e7c2a75

if (serviceConfiguration.isSystemTopicAndTopicLevelPoliciesEnabled()

If the broker without topic policy does not configure topic policy or system topic, getTopicPoliciesBypassSystemTopic will return Optional.empty(), which is just the same behavior with this PR when there are non-retryable errors

[BewareMyPower]

After estimating the risks again, I agree that we should not skip the system topic load exception even if it's non-retryable. Otherwise, the configured system topic policy could be lost even if topicLevelPoliciesEnabled is true. We should prevent the topic from being created anyway.

BTW, there is another issue that the clients would retry even if there is a non-retryable error when loading the system topic. But it can be improved. I might open another PR for it.