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/08/04 09:52:28 UTC

[GitHub] [pulsar] massakam opened a new pull request #7744: [broker] Make resetting cursor in REST API asynchronous

massakam opened a new pull request #7744:
URL: https://github.com/apache/pulsar/pull/7744


   ### Motivation
   
   As mentioned in #7478, `PersistentSubscription#resetCursor()` may not complete for unknown reasons. This blocks `pulsar-web` threads, so it can cause the broker server to be unable to respond to HTTP requests.
   
   ### Modifications
   
   Get the result of `PersistentSubscription#resetCursor()` asynchronously so as not to block `pulsar-web` threads.
   


----------------------------------------------------------------
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 merged pull request #7744: [broker] Make resetting cursor in REST API asynchronous

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


   


----------------------------------------------------------------
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] massakam commented on a change in pull request #7744: [broker] Make resetting cursor in REST API asynchronous

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



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##########
@@ -232,7 +232,6 @@ public void testTerminatePartitionedTopic() {
 
     @Test
     public void testNonPartitionedTopics() {
-        pulsar.getConfiguration().setAllowAutoTopicCreation(false);

Review comment:
       This test fails if `allowAutoTopicCreation` is false.
   
   We are testing if the subscription creation is successful here. However, if `allowAutoTopicCreation` is false, the topic cannot be created and the subscription creation should also fail.
   https://github.com/apache/pulsar/blob/8061c547327b46700c8e66f2d829e258f21a292c/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L1787-L1789
   
   I think it's a mistake to set `allowAutoTopicCreation` to false here. Previously this test passed because the REST API was not able to return the error response correctly.




----------------------------------------------------------------
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] massakam commented on a change in pull request #7744: [broker] Make resetting cursor in REST API asynchronous

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



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/PersistentTopicsTest.java
##########
@@ -232,7 +232,6 @@ public void testTerminatePartitionedTopic() {
 
     @Test
     public void testNonPartitionedTopics() {
-        pulsar.getConfiguration().setAllowAutoTopicCreation(false);

Review comment:
       This test fails if `allowAutoTopicCreation` is false.
   
   We are testing if the subscription creation is successful here. However, if `allowAutoTopicCreation` is false, the topic cannot be created and the subscription creation should also fail.
   https://github.com/apache/pulsar/blob/8061c547327b46700c8e66f2d829e258f21a292c/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L1787-L1789
   
   I think it's a mistake to set `allowAutoTopicCreation` to false 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