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 2022/02/04 01:55:43 UTC

[GitHub] [pulsar] 315157973 opened a new pull request #14114: Support graceful shutdown for Broker

315157973 opened a new pull request #14114:
URL: https://github.com/apache/pulsar/pull/14114


   ### Motivation
   Now the following problems exist when the broker is closing:
   1. When there are many topics on the Broker that is being closed, there will be a large number of concurrent lookup requests, which will eventually cause the Lookup to time out.
   2. When `broker.close()` is running, it directly sets the Topic state to terminated without waiting for the client connection to be closed, resulting in an exception on the client side and obvious perception.
   
   ### Modifications
   1. Add a REST API for graceful shutdown
   2. The rate of unload per second can be controlled in the API
   3. When closing the broker, we can wait for the client connection to close, thereby making the broker switching smoother
   
   ### Documentation
   - [ x ] `no-need-doc` 
   
   
   
   


-- 
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] mattisonchao edited a comment on pull request #14114: Support graceful shutdown for Broker

Posted by GitBox <gi...@apache.org>.
mattisonchao edited a comment on pull request #14114:
URL: https://github.com/apache/pulsar/pull/14114#issuecomment-1029825615


   Hi, @315157973
   
   one question:
   - Since we call ``broker.closeAsync()`` asynchronously, how does the user know if the broker is really closed? because the user always gets 200 after ``unloadNamespaceBundlesGracefully``.
   
   I mean we could enhance the admin side to check broker if is alive after calling ``shutDownBrokerGracefully``. 
   
   Thanks for your answer.


-- 
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] 315157973 commented on pull request #14114: Support graceful shutdown for Broker

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


   > Hi, @315157973
   > 
   > one question:
   > 
   > * Since we call `broker.closeAsync()` asynchronously, how does the user know if the broker is really closed? because the user always gets 200 after `unloadNamespaceBundlesGracefully`.
   > 
   > I mean we could enhance the admin side to check broker if is alive after calling `shutDownBrokerGracefully`.
   > 
   > Thanks for your answer.
   
   If done, the process will exit.


-- 
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] codelipenghui commented on a change in pull request #14114: Support graceful shutdown for Broker

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -446,5 +447,27 @@ private synchronized void deleteDynamicConfigurationOnZk(String configName) {
     public String version() throws Exception {
         return PulsarVersion.getVersion();
     }
+
+    @POST
+    @Path("/shutdown")
+    @ApiOperation(value =
+            "Shutdown broker gracefully.")
+    @ApiResponses(value = {
+            @ApiResponse(code = 204, message = "Execute shutdown command successfully"),
+            @ApiResponse(code = 403, message = "You don't have admin permission to update service-configuration"),
+            @ApiResponse(code = 500, message = "Internal server error")})
+    public void shutDownBrokerGracefully(
+            @QueryParam("maxConcurrentUnloadPerSec") int maxConcurrentUnloadPerSec,
+            @QueryParam("forcedTerminateTopic") @DefaultValue("true") boolean forcedTerminateTopic

Review comment:
       Does the default value is `true` if without `?forcedTerminateTopic=true`?

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -446,5 +447,27 @@ private synchronized void deleteDynamicConfigurationOnZk(String configName) {
     public String version() throws Exception {
         return PulsarVersion.getVersion();
     }
+
+    @POST
+    @Path("/shutdown")
+    @ApiOperation(value =
+            "Shutdown broker gracefully.")
+    @ApiResponses(value = {
+            @ApiResponse(code = 204, message = "Execute shutdown command successfully"),
+            @ApiResponse(code = 403, message = "You don't have admin permission to update service-configuration"),
+            @ApiResponse(code = 500, message = "Internal server error")})
+    public void shutDownBrokerGracefully(
+            @QueryParam("maxConcurrentUnloadPerSec") int maxConcurrentUnloadPerSec,

Review comment:
       Better to add a description here, if the value absent(value=0) means no concurrent limitation.




-- 
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] codelipenghui merged pull request #14114: Support graceful shutdown for Broker

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


   


-- 
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] 315157973 edited a comment on pull request #14114: Support graceful shutdown for Broker

Posted by GitBox <gi...@apache.org>.
315157973 edited a comment on pull request #14114:
URL: https://github.com/apache/pulsar/pull/14114#issuecomment-1029853606


   > Hi, @315157973
   > 
   > one question:
   > 
   > * Since we call `broker.closeAsync()` asynchronously, how does the user know if the broker is really closed? because the user always gets 200 after `unloadNamespaceBundlesGracefully`.
   > 
   > I mean we could enhance the admin side to check broker if is alive after calling `shutDownBrokerGracefully`.
   > 
   > Thanks for your answer.
   
   If done, the process will exit.  We can view the progress of unload through `pulsar-admin brokers namespaces`


-- 
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] mattisonchao commented on pull request #14114: Support graceful shutdown for Broker

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


   Hi, @315157973
   
   one question:
   - Since we call "broker.closeAsync()" asynchronously, how does the user know if the broker is really closed? because user always gets 200 after ``unloadNamespaceBundlesGracefully``.
   
   I mean we could enhance the admin side to check broker if is alive after calling ``shutDownBrokerGracefully``. 
   
   Thanks for your answer.


-- 
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] 315157973 commented on pull request #14114: Support graceful shutdown for Broker

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


   > Can we make this a default behavior for `broker.closeAsync()`? After we called this `shutdown` api, the broker process still exists, right?
   
   `broker.closeAsync()` already has the ability to close gracefully, but it cannot limit the rate of unload, so I added a new API
   
   The broker process should exit, and I should call `PulsarService.close` in my code


-- 
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] 315157973 commented on a change in pull request #14114: Support graceful shutdown for Broker

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -446,5 +447,27 @@ private synchronized void deleteDynamicConfigurationOnZk(String configName) {
     public String version() throws Exception {
         return PulsarVersion.getVersion();
     }
+
+    @POST
+    @Path("/shutdown")
+    @ApiOperation(value =
+            "Shutdown broker gracefully.")
+    @ApiResponses(value = {
+            @ApiResponse(code = 204, message = "Execute shutdown command successfully"),
+            @ApiResponse(code = 403, message = "You don't have admin permission to update service-configuration"),
+            @ApiResponse(code = 500, message = "Internal server error")})
+    public void shutDownBrokerGracefully(
+            @QueryParam("maxConcurrentUnloadPerSec") int maxConcurrentUnloadPerSec,
+            @QueryParam("forcedTerminateTopic") @DefaultValue("true") boolean forcedTerminateTopic

Review comment:
       Yes




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