lhotari commented on code in PR #25070:
URL: https://github.com/apache/pulsar/pull/25070#discussion_r2687309916


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/TopicListService.java:
##########
@@ -71,24 +102,67 @@ public List<String> getMatchingTopics() {
          */
         @Override
         public void accept(String topicName, NotificationType 
notificationType) {
+            if (closed) {
+                return;
+            }
             String partitionedTopicName = 
TopicName.get(topicName).getPartitionedTopicName();
             String domainLessTopicName = 
TopicList.removeTopicDomainScheme(partitionedTopicName);
 
             if (topicsPattern.matches(domainLessTopicName)) {
-                List<String> newTopics;
-                List<String> deletedTopics;
+                List<String> newTopics = Collections.emptyList();
+                List<String> deletedTopics = Collections.emptyList();
                 if (notificationType == NotificationType.Deleted) {
-                    newTopics = Collections.emptyList();
-                    deletedTopics = Collections.singletonList(topicName);
-                    matchingTopics.remove(topicName);
-                } else {
-                    deletedTopics = Collections.emptyList();
+                    if (matchingTopics.remove(topicName)) {
+                        deletedTopics = Collections.singletonList(topicName);
+                    }
+                } else if (matchingTopics.add(topicName)) {
                     newTopics = Collections.singletonList(topicName);
-                    matchingTopics.add(topicName);
                 }
-                String hash = TopicList.calculateHash(matchingTopics);
-                topicListService.sendTopicListUpdate(id, hash, deletedTopics, 
newTopics);
+                if (!newTopics.isEmpty() || !deletedTopics.isEmpty()) {
+                    String hash = TopicList.calculateHash(matchingTopics);
+                    sendTopicListUpdate(hash, deletedTopics, newTopics);
+                }
+            }
+        }
+
+        // sends updates one-by-one so that ordering is retained
+        private synchronized void sendTopicListUpdate(String hash, 
List<String> deletedTopics, List<String> newTopics) {
+            if (closed) {
+                return;
+            }
+            Runnable task = () -> topicListService.sendTopicListUpdate(id, 
hash, deletedTopics, newTopics,
+                    this::sendingCompleted);
+            if (!sendingInProgress) {
+                sendingInProgress = true;
+                executor.execute(task);
+            } else {
+                // if sendTopicListSuccess hasn't completed, add to a queue to 
be executed after it completes
+                if (!sendTopicListUpdateTasks.offer(task)) {
+                    log.warn("Dropping update for watcher id {} matching {} 
since queue is full.", id,
+                            topicsPattern.inputPattern());
+                }

Review Comment:
   I've added some test coverage to TopicListService level for refreshing all 
topics when the queue overflows. To fix the problem, it will be necessary to 
also address #25020. I think it's better to have that in a separate PR.



-- 
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]

Reply via email to