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/04/23 07:25:28 UTC

[GitHub] [pulsar] 315157973 opened a new pull request #10337: Fix CPU 100% in some cases

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


   ### Motivation
   When deleting the namespace, the namespace Policies will be marked as deleted.
   This will trigger topic's `onPoliciesUpdate`
   However, in onPoliciesUpdate, the data of the Policies node on zk will be read, such as: `checkReplicationAndRetryOnFailure`
   Due to the deletion of the namespace, the zk node may no longer exist at this time. 
   Failure to read data will trigger infinite retries.
   https://github.com/apache/pulsar/blob/e970c2947aff9231202ab72bdbad047d85c55633/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L1175-L1193
   
   If there are many topics, there will be a short-term CPU spike
   
   ![image](https://user-images.githubusercontent.com/9758905/115834541-ebc32480-a447-11eb-887a-95c4a3d1adf1.png)
   
   
   ### Modifications
   
   
   ### Verifying 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] eolivelli commented on pull request #10337: Fix CPU 100% when deleting namespace

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


   @315157973 thank you for porting this patch to branch-2.7.
   I have included it in 2.7.2rc1


-- 
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] eolivelli commented on a change in pull request #10337: Fix CPU 100% when deleting namespace

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



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PersistentTopicTest.java
##########
@@ -156,4 +161,28 @@ public void testUnblockStuckSubscription() throws Exception {
         msg = consumer2.receive(5, TimeUnit.SECONDS);
         assertNotNull(msg);
     }
+
+    @Test
+    public void testDeleteNamespaceInfiniteRetry() throws Exception {
+        //init namespace
+        final String myNamespace = "prop/ns" + UUID.randomUUID();
+        admin.namespaces().createNamespace(myNamespace, Sets.newHashSet("test"));
+        final String topic = "persistent://" + myNamespace + "/testDeleteNamespaceInfiniteRetry";
+        conf.setForceDeleteNamespaceAllowed(true);
+        //init topic and policies
+        pulsarClient.newProducer().topic(topic).create().close();
+        admin.namespaces().setMaxConsumersPerTopic(myNamespace, 0);
+        Awaitility.await().atMost(3, TimeUnit.SECONDS).until(()
+                -> admin.namespaces().getMaxConsumersPerTopic(myNamespace) == 0);
+
+        PersistentTopic persistentTopic =
+                spy((PersistentTopic) pulsar.getBrokerService().getTopicIfExists(topic).get().get());
+
+        admin.namespaces().deleteNamespace(myNamespace, true);
+
+        Policies policies = new Policies();
+        policies.deleted = true;
+        persistentTopic.onPoliciesUpdate(policies);
+        verify(persistentTopic, times(0)).checkReplicationAndRetryOnFailure();

Review comment:
       CN we add a positive test, that verifies that this method is called in case of non deleted Policies?




-- 
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 #10337: Fix CPU 100% when deleting namespace

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


   


-- 
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 #10337: Fix CPU 100% when deleting namespace

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


   


-- 
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] eolivelli commented on pull request #10337: Fix CPU 100% when deleting namespace

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


   @315157973 
   this patch does not apply to branch-2.7
   can you please create a new PR for branch-2.7 ?


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