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 2020/06/04 02:10:41 UTC

[GitHub] [pulsar] zhanghaou opened a new pull request #7093: Add control of single topic gc in inactive state.

zhanghaou opened a new pull request #7093:
URL: https://github.com/apache/pulsar/pull/7093


   **Motivation**
   The brokerDeleteInactiveTopicsEnabled parameter is effect to all Topics, but in some cases like topic created by protocol handler, we need keep some topics not be deleted even if they're in inactive state.
   
   **Modifications**
   Add deleteWhileInactive parameter in AbstractTopic, and when create a new topic,  we can call topic.setDeleteWhileInactive(false) to keep the topic not be deleted by GC.


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

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



[GitHub] [pulsar] zhanghaou closed pull request #7093: Add control of single topic gc in inactive state.

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


   


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

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



[GitHub] [pulsar] zhanghaou commented on a change in pull request #7093: Add control of single topic gc in inactive state.

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -63,6 +63,9 @@
 
     protected volatile boolean isFenced;
 
+    // When set to false, this inactive topic can not be deleted
+    protected boolean brokerDeleteInactiveTopicsEnabled;

Review comment:
       Done.




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

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



[GitHub] [pulsar] zhanghaou commented on pull request #7093: Add control of single topic gc in inactive state.

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


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] codelipenghui commented on pull request #7093: Add control of single topic gc in inactive state.

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


   ping @merlimat please help review this PR again.


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

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



[GitHub] [pulsar] merlimat commented on a change in pull request #7093: Add control of single topic gc in inactive state.

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -423,5 +428,13 @@ public long getBytesOutCounter() {
         return getStats(false).bytesOutCounter;
     }
 
+    public boolean isBrokerDeleteInactiveTopicsEnabled() {
+        return brokerDeleteInactiveTopicsEnabled;
+    }
+
+    public void setBrokerDeleteInactiveTopicsEnabled(boolean brokerDeleteInactiveTopicsEnabled) {

Review comment:
       How is this supposed to be activated? 
   
   Instead, we need a namespace policy, attached to https://pulsar.apache.org/admin-rest-api/?version=2.5.2#operation/setAutoTopicCreation that adds the auto-deletion override configuration, on top of the auto-creation




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

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



[GitHub] [pulsar] codelipenghui commented on pull request #7093: Add control of single topic gc in inactive state.

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


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] sijie commented on pull request #7093: Add control of single topic gc in inactive state.

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


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] codelipenghui commented on pull request #7093: Add control of single topic gc in inactive state.

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


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] zhanghaou commented on pull request #7093: Add control of single topic gc in inactive state.

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


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] codelipenghui merged pull request #7093: Add control of single topic gc in inactive state.

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #7093:
URL: https://github.com/apache/pulsar/pull/7093


   


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

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



[GitHub] [pulsar] codelipenghui commented on a change in pull request #7093: Add control of single topic gc in inactive state.

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -423,5 +428,13 @@ public long getBytesOutCounter() {
         return getStats(false).bytesOutCounter;
     }
 
+    public boolean isBrokerDeleteInactiveTopicsEnabled() {
+        return brokerDeleteInactiveTopicsEnabled;
+    }
+
+    public void setBrokerDeleteInactiveTopicsEnabled(boolean brokerDeleteInactiveTopicsEnabled) {

Review comment:
       @merlimat This is used by the protocol handlers who want to ensure some topics under the namespace does not delete by the broker. But do not want to change the namespace level policy. Now, we don't have a topic level policy, without this change, we must create a virtual producer or consume.

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -63,6 +63,9 @@
 
     protected volatile boolean isFenced;
 
+    // When set to false, this inactive topic can not be deleted
+    protected boolean brokerDeleteInactiveTopicsEnabled;

Review comment:
       I think it's better to named `deleteWhileInactive`.

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -830,6 +830,10 @@ public boolean isActive() {
 
     @Override
     public void checkGC(int maxInactiveDurationInSec, InactiveTopicDeleteMode deleteMode) {
+        if (brokerDeleteInactiveTopicsEnabled) {

Review comment:
       ```suggestion
           if (! brokerDeleteInactiveTopicsEnabled) {
   ```

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -1636,6 +1629,10 @@ private boolean hasBacklogs() {
 
     @Override
     public void checkGC(int maxInactiveDurationInSec, InactiveTopicDeleteMode deleteMode) {
+        if (brokerDeleteInactiveTopicsEnabled) {

Review comment:
       ```suggestion
           if (!brokerDeleteInactiveTopicsEnabled) {
   ```




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

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



[GitHub] [pulsar] zhanghaou commented on pull request #7093: Add control of single topic gc in inactive state.

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


   /pulsarbot run-failure-checks


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

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