lhotari commented on code in PR #24833:
URL: https://github.com/apache/pulsar/pull/24833#discussion_r2503808541
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/TopicListService.java:
##########
@@ -101,6 +112,7 @@ public void accept(String topicName, NotificationType
notificationType) {
private final boolean enableSubscriptionPatternEvaluation;
private final int maxSubscriptionPatternLength;
private final ConcurrentLongHashMap<CompletableFuture<TopicListWatcher>>
watchers;
+ private final Backoff retryBackoff;
public TopicListService(PulsarService pulsar, ServerCnx connection,
Review Comment:
@poorbarcode It appears that previous changes have broke the topic list
watcher functionality. In the current Pulsar client code, the
CommandWatchTopicListSuccess response is ignored:
https://github.com/apache/pulsar/blame/1300f0a4b1c015e278ff321d58c12a7e85ba12e7/pulsar-client/src/main/java/org/apache/pulsar/client/impl/TopicListWatcher.java#L136-L152
This was changed in #21853 while fixing the lost topic list events issue.
While working on the code, I found the actual root cause that causes topic list
events to be lost. I'll fix that in a separate PR.
What I'll do now is that I'll split the Topic list watcher memory limiting
to another PR from this current PR. That is useful since the PR to fix the
topic list events issue is not related to PIP-442. After that is fixed, it
makes sense to continue adding PIP-442 to cover also topic list events.
This way we can also make progress on getting this current PR merged,
without the topic list watcher limiting changes.
--
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]
