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/08/09 11:12:43 UTC

[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16540: [test]Fix Flaky-test: BrokerServiceTest.testLookupThrottlingForClientByClient

AnonHxy commented on code in PR #16540:
URL: https://github.com/apache/pulsar/pull/16540#discussion_r941204324


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BrokerServiceTest.java:
##########
@@ -979,7 +981,32 @@ public void testLookupThrottlingForClientByClient() throws Exception {
         EventLoopGroup eventLoop = EventLoopUtil.newEventLoopGroup(20, false,
                 new DefaultThreadFactory("test-pool", Thread.currentThread().isDaemon()));
         long reqId = 0xdeadbeef;
-        try (ConnectionPool pool = new ConnectionPool(conf, eventLoop)) {
+
+        Semaphore clientCnxSemaphore = new Semaphore(1);
+        AtomicInteger count = new AtomicInteger(2);
+        try (ConnectionPool pool = new ConnectionPool(conf, eventLoop, () -> new ClientCnx(conf, eventLoop) {
+            @Override
+            protected void handleLookupResponse(CommandLookupTopicResponse lookupResult) {
+                try {
+                    clientCnxSemaphore.acquire();

Review Comment:
   But if so, I think there still exists a small chance that  all of the three requests can request success, which will make the test fail. 
   
   Because they are request concurrently,  then one of the requests, e.g., request3, still have the chance  that begin sending request after request1 and request2 completed @poorbarcode 



##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BrokerServiceTest.java:
##########
@@ -979,7 +981,32 @@ public void testLookupThrottlingForClientByClient() throws Exception {
         EventLoopGroup eventLoop = EventLoopUtil.newEventLoopGroup(20, false,
                 new DefaultThreadFactory("test-pool", Thread.currentThread().isDaemon()));
         long reqId = 0xdeadbeef;
-        try (ConnectionPool pool = new ConnectionPool(conf, eventLoop)) {
+
+        Semaphore clientCnxSemaphore = new Semaphore(1);
+        AtomicInteger count = new AtomicInteger(2);

Review Comment:
   After I thinking about this, there is no need to add `count` @poorbarcode , I will remove it.



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