chenhongSZ commented on PR #21212: URL: https://github.com/apache/pulsar/pull/21212#issuecomment-1767480641
> > I'm sorry, but I don't think this change suits me. > > There are some reasons are as follows: > > > > 1. Apache Pulsar has the `broker`/`namespace`/`topic` level policy. The global topic policy is just a particular topic policy based on the geo-replication. We should refrain from introducing this concept into the topic entity. > > 2. We should encapsulate this concept into topic policy service, which can decide to give the topic a global or local policy because the topic doesn't care if the policy is global or not. > > > > Therefore, I would like to let `[SystemTopicBasedTopicPoliciesService.java](https://github.com/apache/pulsar/pull/21212/files#diff-9d2948d863c111e4be6d508a1c573667a1326b98c4314e917ba9e344bb61dc27)` decide to update global or local policy to the topic to avoid `isGlobal` param everywhere. > > WDYT? /cc @codelipenghui @Technoboy- @poorbarcode > > I agree, It is indeed we should encapsulate this concept into topic policy service. I will make some changes as below: > > 1. rollback `PolicyHierarchyValue` to its previous state (use `topicValue` replace `localTopicValue` and `globalTopicValue`) > 2. merge the local and global topic policies into one `TopicPolicies` at `SystemTopicBasedTopicPoliciesService` before init topic and notify topic. therefore `org.apache.pulsar.broker.service.persistent.PersistentTopic#onUpdate` method doesn't care if the policy is global or not. but the `org.apache.pulsar.common.policies.data.TopicPolicies#isGlobal` becomes meaningless in the topic context. so we should not use this variable in the topic context. > Any suggestions? those changes have been done in my latest commit, PTAL, thanks @mattisonchao -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
