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/03/05 09:55:47 UTC

[GitHub] [pulsar] murong00 opened a new pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

murong00 opened a new pull request #9819:
URL: https://github.com/apache/pulsar/pull/9819


   ### Motivation
   
   Fixes #9790
   
   ### Modifications
   
   - Add new configs below and keep the default value to `false`:
   ```
   forceDeleteTenantAllowed=false
   forceDeleteNamespaceAllowed=false
   ```
   - Add unit tests for this change.


----------------------------------------------------------------
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] murong00 commented on a change in pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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



##########
File path: site2/docs/reference-configuration.md
##########
@@ -185,6 +185,8 @@ internalListenerName|Specify the internal listener name for the broker.<br><br>*
 |brokerDeleteInactiveTopicsFrequencySeconds|  How often to check for inactive topics  |60|
 | brokerDeleteInactiveTopicsMode | Set the mode to delete inactive topics. <li> `delete_when_no_subscriptions`: delete the topic which has no subscriptions or active producers. <li> `delete_when_subscriptions_caught_up`: delete the topic whose subscriptions have no backlogs and which has no active producers or consumers. | `delete_when_no_subscriptions` |
 | brokerDeleteInactiveTopicsMaxInactiveDurationSeconds | Set the maximum duration for inactive topics. If it is not specified, the `brokerDeleteInactiveTopicsFrequencySeconds` parameter is adopted. | N/A |
+|forceDeleteTenantAllowed| Allow forced deletion of tenants.|false|

Review comment:
       Done.

##########
File path: conf/broker.conf
##########
@@ -165,6 +165,12 @@ brokerDeleteInactivePartitionedTopicMetadataEnabled=false
 # Topics that are inactive for longer than this value will be deleted
 brokerDeleteInactiveTopicsMaxInactiveDurationSeconds=
 
+# Allow forced deletion of tenants.

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] murong00 commented on pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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


   /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] murong00 commented on pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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


   /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] Anonymitaet commented on a change in pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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



##########
File path: conf/broker.conf
##########
@@ -165,6 +165,12 @@ brokerDeleteInactivePartitionedTopicMetadataEnabled=false
 # Topics that are inactive for longer than this value will be deleted
 brokerDeleteInactiveTopicsMaxInactiveDurationSeconds=
 
+# Allow forced deletion of tenants.

Review comment:
       ```suggestion
   # Allow you to delete a tenant forcefully.
   ```

##########
File path: site2/docs/reference-configuration.md
##########
@@ -185,6 +185,8 @@ internalListenerName|Specify the internal listener name for the broker.<br><br>*
 |brokerDeleteInactiveTopicsFrequencySeconds|  How often to check for inactive topics  |60|
 | brokerDeleteInactiveTopicsMode | Set the mode to delete inactive topics. <li> `delete_when_no_subscriptions`: delete the topic which has no subscriptions or active producers. <li> `delete_when_subscriptions_caught_up`: delete the topic whose subscriptions have no backlogs and which has no active producers or consumers. | `delete_when_no_subscriptions` |
 | brokerDeleteInactiveTopicsMaxInactiveDurationSeconds | Set the maximum duration for inactive topics. If it is not specified, the `brokerDeleteInactiveTopicsFrequencySeconds` parameter is adopted. | N/A |
+|forceDeleteTenantAllowed| Allow forced deletion of tenants.|false|

Review comment:
       ```suggestion
   |forceDeleteTenantAllowed| Allow you to delete a tenant forcefully. |false|
   ```

##########
File path: conf/broker.conf
##########
@@ -165,6 +165,12 @@ brokerDeleteInactivePartitionedTopicMetadataEnabled=false
 # Topics that are inactive for longer than this value will be deleted
 brokerDeleteInactiveTopicsMaxInactiveDurationSeconds=
 
+# Allow forced deletion of tenants.
+forceDeleteTenantAllowed=false
+
+# Allow forced deletion of namespaces.

Review comment:
       ```suggestion
   # Allow you to delete a namespace forcefully.
   ```

##########
File path: site2/docs/reference-configuration.md
##########
@@ -185,6 +185,8 @@ internalListenerName|Specify the internal listener name for the broker.<br><br>*
 |brokerDeleteInactiveTopicsFrequencySeconds|  How often to check for inactive topics  |60|
 | brokerDeleteInactiveTopicsMode | Set the mode to delete inactive topics. <li> `delete_when_no_subscriptions`: delete the topic which has no subscriptions or active producers. <li> `delete_when_subscriptions_caught_up`: delete the topic whose subscriptions have no backlogs and which has no active producers or consumers. | `delete_when_no_subscriptions` |
 | brokerDeleteInactiveTopicsMaxInactiveDurationSeconds | Set the maximum duration for inactive topics. If it is not specified, the `brokerDeleteInactiveTopicsFrequencySeconds` parameter is adopted. | N/A |
+|forceDeleteTenantAllowed| Allow forced deletion of tenants.|false|
+|forceDeleteNamespaceAllowed| Allow forced deletion of namespaces.|false|

Review comment:
       ```suggestion
   |forceDeleteNamespaceAllowed| Allow you to delete a namespace forcefully. |false|
   ```




----------------------------------------------------------------
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] murong00 commented on pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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


   /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 #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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


   


----------------------------------------------------------------
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] murong00 commented on a change in pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java
##########
@@ -290,6 +290,12 @@ protected void internalDeleteTenant(AsyncResponse asyncResponse, String tenant)
     }
 
     protected void internalDeleteTenantForcefully(AsyncResponse asyncResponse, String tenant) {
+        if (!pulsar().getConfiguration().isForceDeleteTenantAllowed()) {
+            asyncResponse.resume(
+                    new RestException(Status.FORBIDDEN, "Broker doesn't allow forced deletion of tenants"));

Review comment:
       Thanks for the tips, 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] murong00 commented on pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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


   @Anonymitaet Added the docs accordingly, please help to take a look when you are available.


----------------------------------------------------------------
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 #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TenantsBase.java
##########
@@ -290,6 +290,12 @@ protected void internalDeleteTenant(AsyncResponse asyncResponse, String tenant)
     }
 
     protected void internalDeleteTenantForcefully(AsyncResponse asyncResponse, String tenant) {
+        if (!pulsar().getConfiguration().isForceDeleteTenantAllowed()) {
+            asyncResponse.resume(
+                    new RestException(Status.FORBIDDEN, "Broker doesn't allow forced deletion of tenants"));

Review comment:
       The @ApiReponse use 403 to indicate if the request has admin permission, I think 405 is more reasonable here?




----------------------------------------------------------------
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] murong00 removed a comment on pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

Posted by GitBox <gi...@apache.org>.
murong00 removed a comment on pull request #9819:
URL: https://github.com/apache/pulsar/pull/9819#issuecomment-792482329


   /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] Anonymitaet commented on pull request #9819: [Issue 9790] Support disabled the tenants/namespaces force deletion in the broker.conf

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


   @murong00 thanks for your great work! Shall the changes be documented to the [broker -configuration](http://pulsar.incubator.apache.org/docs/en/next/reference-configuration/#broker) section?
   If so, could you please help add the docs accordingly? Thanks


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