BewareMyPower commented on PR #25707: URL: https://github.com/apache/pulsar/pull/25707#issuecomment-4473939439
@dao-jun I've updated the document for why the implementation adopts a different path. > The return value of registerListener (a registration handle) is discarded. close() only clears the local listeners map and invalidates caches, but never unregisters these metadata-store listeners. After close(), the service can still receive and process notifications (the closed flag guards most paths, but the listener lambdas themselves are still held by the metadata store). Consider capturing and closing the registration handles in close(). Regarding the previous comment, it seems to be wrong. Firstly, `registerListener` does not return any handle, see https://github.com/apache/pulsar/blob/8f9f5b49d631e235e86d79e48a63722e74db4413/pulsar-metadata/src/main/java/org/apache/pulsar/metadata/api/MetadataStore.java#L249 Secondly, it follows the similar pattern of https://github.com/apache/pulsar/blob/8f9f5b49d631e235e86d79e48a63722e74db4413/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L381-L384 Though this PR has registered two listeners with a parameter that indicates if the policy is global so that we don't need to check if the notification path starts with `GLOBAL_POLICIES_ROOT` or `LOCAL_POLICIES_ROOT`. Could you double check the review comments if they are generated from LLM? -- 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]
