apache / apache/pulsar

[Bug] Topic policy is updated incorrectly when continuously use pulsar-admin to set policy

Open
#21,024 1 comment 0 reactions 1 assignee Claimed by @TakaHiR07 View on GitHub
Stale type/bug
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

### Search before asking

- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.

### Version

master branch

### Minimal reproduce step

Two incorrect case of topic policy

case1:
1. topic do not have topicPolicy
2. use pulsar-admin to set retention for topic
3. use pulsar-admin to set message ttl for topic

case 2:
1. topic have policy
2. use pulsar-admin to set retention for topic, update retentionTime to 1d
3. use pulsar-admin to set retention for topic, update retentionTime to 2d
4. use pulsar-admin to set message ttl for topic

### What did you expect to see?

Both retention and ttl is set correctly

### What did you see instead?

For case 1:
- Only ttl is set, retention is not set correctly.
![image](https://github.com/apache/pulsar/assets/13505225/bd0f24a5-ac22-4b04-93ba-c4f86bcb2207)

For case 2:
- ttl is set, retentionTime is set as 1d, but not 2d.

### Anything else?

The code of set/get topic policy result in this case.

In SystemTopicBasedTopicPoliciesService, policiesCache would be update in refreshTopicPoliciesCache(). The process is :
1. systemTopicReader successfully read the next event message
2. update the value in policiesCache
3. notifyListener() to actually update topic policy in PersistentTopic#onUpdate()

https://github.com/apache/pulsar/blob/4a9fec693b90fcb76aea9a98a94de31608b56bab/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java#L470-L476

But the operation of PersistentTopicsBase#internalSetRetention() is:
1. get the current topicPolicy from policiesCache. If it is not exist in policiesCache, construct a new topicPolicy.
2. Then do setRetention. It may change the policy value in policiesCache.
3. send updatePolicyEvent msg to __change_events. If send success, response success to client.

https://github.com/apache/pulsar/blob/4a9fec693b90fcb76aea9a98a94de31608b56bab/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L3467-L3493

Therefore, it would cause the above two bad case
1. If topic do not have topic policy, after use pulsar-admin to setRetention success, the policiesCache actually is not update. policiesCache would be updated until systemTopicReader read the next msg in __change_events. So we then use pulsar-admin to setMessageTTL, maybe get null topicPolicy from policiesCache.
2. After twice setRetention operation, we update retentionTime to 1d and then 2d, the policiesCache would be updated four times. So we then setMessageTTL, maybe get retentionTime is 1d from policiesCache.

### Are you willing to submit a PR?

- [X] I'm willing to submit a PR!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.