You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/02 11:20:53 UTC

[GitHub] [pulsar] shibd opened a new pull request #11898: [Broker] no-persistent topic support terminate

shibd opened a new pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898


   #11834 


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] congbobo184 commented on a change in pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on a change in pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#discussion_r701032186



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -328,6 +329,16 @@ public void removeProducer(Producer producer) {
         return delete(false, false, false);
     }
 
+    @Override
+    public CompletableFuture<MessageId> terminate() {
+        CompletableFuture<MessageId> future = new CompletableFuture<>();
+        producers.values().forEach(Producer::disconnect);

Review comment:
       why not disconnect sub? 




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] gaozhangmin commented on a change in pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#discussion_r701128828



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -328,6 +329,16 @@ public void removeProducer(Producer producer) {
         return delete(false, false, false);
     }
 
+    @Override
+    public CompletableFuture<MessageId> terminate() {
+        CompletableFuture<MessageId> future = new CompletableFuture<>();
+        producers.values().forEach(Producer::disconnect);

Review comment:
       I think non-persistent termination should behave like persistent




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] shibd commented on pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
shibd commented on pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#issuecomment-911869073


   > The problem with non-persistent topics is that the termination will be lost at the next broker restart or topic failover so it would not make much practical sense.
   
   Got it, I will close this 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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] shibd closed pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
shibd closed pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898


   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] shibd commented on a change in pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
shibd commented on a change in pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#discussion_r701076309



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -328,6 +329,16 @@ public void removeProducer(Producer producer) {
         return delete(false, false, false);
     }
 
+    @Override
+    public CompletableFuture<MessageId> terminate() {
+        CompletableFuture<MessageId> future = new CompletableFuture<>();
+        producers.values().forEach(Producer::disconnect);

Review comment:
       Thanks for the review. I think still consumer to drain existing messages in backlog after the termination.Disconnect subscribe will close existing consumers.
   
   May need to call `Subscription#close()` that no longer accepts new consumers
   
   ```
       @Override
       public CompletableFuture<Void> close() {
           IS_FENCED_UPDATER.set(this, TRUE);
           return CompletableFuture.completedFuture(null);
       }
   ```
   
   Do you have any better suggestions.
   




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] shibd commented on pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
shibd commented on pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#issuecomment-911681314


   > Could you please add a test for it?
   
   Ok, I'll add it


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] shibd commented on a change in pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
shibd commented on a change in pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#discussion_r701076309



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -328,6 +329,16 @@ public void removeProducer(Producer producer) {
         return delete(false, false, false);
     }
 
+    @Override
+    public CompletableFuture<MessageId> terminate() {
+        CompletableFuture<MessageId> future = new CompletableFuture<>();
+        producers.values().forEach(Producer::disconnect);

Review comment:
       Thanks for the review. I think still consumer to drain existing messages in backlog after the termination.Disconnect subscribe will close existing consumers.
   
   May need to call `Subscription#close()` that no longer accepts new consumers
   
   ```
       @Override
       public CompletableFuture<Void> close() {
           IS_FENCED_UPDATER.set(this, TRUE);
           return CompletableFuture.completedFuture(null);
       }
   ```
   
   Do you have any better suggestions?
   




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] congbobo184 commented on a change in pull request #11898: [Broker] no-persistent topic support terminate

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on a change in pull request #11898:
URL: https://github.com/apache/pulsar/pull/11898#discussion_r701118746



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -328,6 +329,16 @@ public void removeProducer(Producer producer) {
         return delete(false, false, false);
     }
 
+    @Override
+    public CompletableFuture<MessageId> terminate() {
+        CompletableFuture<MessageId> future = new CompletableFuture<>();
+        producers.values().forEach(Producer::disconnect);

Review comment:
       do we need retain existing consumer when non-persistent topic terminate? @gaoran10 @codelipenghui /cc 




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org