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/06/18 21:22:37 UTC

[GitHub] [pulsar] 315157973 commented on a change in pull request #10963: [Broker] Fix create partitioned topic in replicated namespace

315157973 commented on a change in pull request #10963:
URL: https://github.com/apache/pulsar/pull/10963#discussion_r654156171



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
##########
@@ -1004,6 +1004,59 @@ public void testTopicReplicatedAndProducerCreate(String topicPrefix, String topi
         nonPersistentProducer2.close();
     }
 
+    @Test
+    public void createPartitionedTopicTest() throws Exception {
+        final String cluster1 = pulsar1.getConfig().getClusterName();
+        final String cluster2 = pulsar2.getConfig().getClusterName();
+        final String cluster3 = pulsar3.getConfig().getClusterName();
+        final String namespace = randomSuffixString("pulsar/ns", 5);
+
+        final String persistentPartitionedTopic =
+                randomSuffixString("persistent://" + namespace + "/partitioned", 5);
+        final String persistentNonPartitionedTopic =
+                randomSuffixString("persistent://" + namespace + "/non-partitioned", 5);
+        final String nonPersistentPartitionedTopic =
+                randomSuffixString("non-persistent://" + namespace + "/partitioned", 5);
+        final String nonPersistentNonPartitionedTopic =
+                randomSuffixString("non-persistent://" + namespace + "/non-partitioned", 5);
+        final int numPartitions = 3;
+
+        admin1.namespaces().createNamespace(namespace, Sets.newHashSet(cluster1, cluster2, cluster3));
+        admin1.namespaces().setNamespaceReplicationClusters(namespace, Sets.newHashSet("r1", "r2", "r3"));
+
+        admin1.topics().createPartitionedTopic(persistentPartitionedTopic, numPartitions);
+        admin1.topics().createPartitionedTopic(nonPersistentPartitionedTopic, numPartitions);
+        admin1.topics().createNonPartitionedTopic(persistentNonPartitionedTopic);
+        admin1.topics().createNonPartitionedTopic(nonPersistentNonPartitionedTopic);
+
+        List<String> partitionedTopicList = admin1.topics().getPartitionedTopicList(namespace);
+        Assert.assertTrue(partitionedTopicList.contains(persistentPartitionedTopic));
+        Assert.assertTrue(partitionedTopicList.contains(nonPersistentPartitionedTopic));
+
+        // wait non-partitioned topics replicators created finished
+        Thread.sleep(1000);

Review comment:
       Flaky test is easy to appear in this way, can we use Awaitility?
   `admin.topics().getList().size > x` can be used as the judgment condition

##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
##########
@@ -1004,6 +1004,59 @@ public void testTopicReplicatedAndProducerCreate(String topicPrefix, String topi
         nonPersistentProducer2.close();
     }
 
+    @Test
+    public void createPartitionedTopicTest() throws Exception {
+        final String cluster1 = pulsar1.getConfig().getClusterName();
+        final String cluster2 = pulsar2.getConfig().getClusterName();
+        final String cluster3 = pulsar3.getConfig().getClusterName();
+        final String namespace = randomSuffixString("pulsar/ns", 5);
+
+        final String persistentPartitionedTopic =
+                randomSuffixString("persistent://" + namespace + "/partitioned", 5);
+        final String persistentNonPartitionedTopic =
+                randomSuffixString("persistent://" + namespace + "/non-partitioned", 5);
+        final String nonPersistentPartitionedTopic =
+                randomSuffixString("non-persistent://" + namespace + "/partitioned", 5);
+        final String nonPersistentNonPartitionedTopic =
+                randomSuffixString("non-persistent://" + namespace + "/non-partitioned", 5);
+        final int numPartitions = 3;
+
+        admin1.namespaces().createNamespace(namespace, Sets.newHashSet(cluster1, cluster2, cluster3));
+        admin1.namespaces().setNamespaceReplicationClusters(namespace, Sets.newHashSet("r1", "r2", "r3"));
+
+        admin1.topics().createPartitionedTopic(persistentPartitionedTopic, numPartitions);
+        admin1.topics().createPartitionedTopic(nonPersistentPartitionedTopic, numPartitions);
+        admin1.topics().createNonPartitionedTopic(persistentNonPartitionedTopic);
+        admin1.topics().createNonPartitionedTopic(nonPersistentNonPartitionedTopic);
+
+        List<String> partitionedTopicList = admin1.topics().getPartitionedTopicList(namespace);
+        Assert.assertTrue(partitionedTopicList.contains(persistentPartitionedTopic));
+        Assert.assertTrue(partitionedTopicList.contains(nonPersistentPartitionedTopic));
+
+        // wait non-partitioned topics replicators created finished
+        Thread.sleep(1000);

Review comment:
       Flaky test is easy to appear in this way, can we use Awaitility?
   `admin.topics(). getPartitionedTopicList().size > x` can be used as the judgment condition




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