Flaky-test: RackAwareTest.testPlacement (apache#11370)

* Flaky-test: RackAwareTest.testPlacement (apache#11349)

* repalce setup with configurePulsar

* Fix cleanup method (annotation cannot be removed)

* Increase zooKeeperOperationTimeoutSeconds to 10 seconds

* Move RackAwareTest to quarantine group to unblock CI

- fixing RackAwareTest requires fixing the bug in ZooKeeperCache class
  which seems to get into a deadlock

Co-authored-by: Lari Hotari <[email protected]>

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BkEnsemblesTestBase.java

@@ -60,7 +60,7 @@ public BkEnsemblesTestBase(int numberOfBookies) {
         this.numberOfBookies = numberOfBookies;
     }
 
-    protected void configurePulsar(ServiceConfiguration config) {
+    protected void configurePulsar(ServiceConfiguration config) throws Exception {
         //overridable by subclasses
     }
 
@@ -87,7 +87,7 @@ protected void setup() throws Exception {
             config.setManagedLedgerMinLedgerRolloverTimeMinutes(0);
             config.setAdvertisedAddress("127.0.0.1");
             config.setAllowAutoTopicCreationType("non-partitioned");
-            config.setZooKeeperOperationTimeoutSeconds(1);
+            config.setZooKeeperOperationTimeoutSeconds(10);
             config.setNumIOThreads(1);
             Properties properties = new Properties();
             properties.put("bookkeeper_numWorkerThreads", "1");

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/RackAwareTest.java

@@ -19,27 +19,33 @@
 package org.apache.pulsar.broker.service;
 
 import static org.testng.Assert.assertTrue;
-
+import com.google.gson.Gson;
 import java.io.File;
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.List;
-
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.stream.Collectors;
 import org.apache.bookkeeper.client.BookKeeper;
 import org.apache.bookkeeper.client.BookKeeper.DigestType;
 import org.apache.bookkeeper.client.LedgerHandle;
 import org.apache.bookkeeper.conf.ServerConfiguration;
 import org.apache.bookkeeper.net.BookieId;
 import org.apache.bookkeeper.proto.BookieServer;
 import org.apache.bookkeeper.stats.NullStatsLogger;
+import org.apache.pulsar.broker.ServiceConfiguration;
 import org.apache.pulsar.common.policies.data.BookieInfo;
+import org.apache.pulsar.zookeeper.ZkBookieRackAffinityMapping;
+import org.assertj.core.util.Lists;
+import org.awaitility.Awaitility;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.testng.annotations.AfterClass;
-import org.testng.annotations.BeforeClass;
+import org.testng.annotations.AfterMethod;
 import org.testng.annotations.Test;
 
-@Test(groups = "broker")
+@Test(groups = "quarantine")
 public class RackAwareTest extends BkEnsemblesTestBase {
 
     private static final int NUM_BOOKIES = 6;
@@ -50,10 +56,8 @@ public RackAwareTest() {
         super(0);
     }
 
-    @BeforeClass(alwaysRun = true)
-    protected void setup() throws Exception {
-        super.setup();
-
+    @Override
+    protected void configurePulsar(ServiceConfiguration config) throws Exception {
         // Start bookies with specific racks
         for (int i = 0; i < NUM_BOOKIES; i++) {
             File bkDataDir = Files.createTempDirectory("bk" + Integer.toString(i) + "test").toFile();
@@ -79,7 +83,8 @@ protected void setup() throws Exception {
 
     }
 
-    @AfterClass(alwaysRun = true)
+    @Override
+    @AfterMethod(alwaysRun = true)
     protected void cleanup() throws Exception {
         super.cleanup();
 
@@ -92,6 +97,7 @@ protected void cleanup() throws Exception {
 
     @Test
     public void testPlacement() throws Exception {
+        final String group = "default";
         for (int i = 0; i < NUM_BOOKIES; i++) {
             String bookie = bookies.get(i).getLocalAddress().toString();
 
@@ -102,20 +108,31 @@ public void testPlacement() throws Exception {
                     .hostname("bookie-" + (i + 1))
                     .build();
             log.info("setting rack for bookie at {} -- {}", bookie, bi);
-            admin.bookies().updateBookieRackInfo(bookie, "default", bi);
+            admin.bookies().updateBookieRackInfo(bookie, group, bi);
         }
 
         // Make sure the racks cache gets updated through the ZK watch
-        Thread.sleep(1000);
+        Awaitility.await().untilAsserted(() -> {
+            byte[] data = bkEnsemble.getZkClient()
+                    .getData(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, false, null);
+            TreeMap<String, Map<String, Map<String, String>>> rackInfoMap =
+                    new Gson().fromJson(new String(data), TreeMap.class);
+            assertTrue(rackInfoMap.get(group).size() == NUM_BOOKIES);
+            Set<String> racks = rackInfoMap.values().stream()
+                    .map(Map::values)
+                    .flatMap(bookieId -> bookieId.stream().map(rackInfo -> rackInfo.get("rack")))
+                    .collect(Collectors.toSet());
+            assertTrue(racks.containsAll(Lists.newArrayList("rack-1", "rack-2")));
+        });
 
         BookKeeper bkc = this.pulsar.getBookKeeperClient();
 
         // Create few ledgers and verify all of them should have a copy in the first bookie
-        BookieId fistBookie = bookies.get(0).getBookieId();
+        BookieId firstBookie = bookies.get(0).getBookieId();
         for (int i = 0; i < 100; i++) {
             LedgerHandle lh = bkc.createLedger(2, 2, DigestType.DUMMY, new byte[0]);
             log.info("Ledger: {} -- Ensemble: {}", i, lh.getLedgerMetadata().getEnsembleAt(0));
-            assertTrue(lh.getLedgerMetadata().getEnsembleAt(0).contains(fistBookie),
+            assertTrue(lh.getLedgerMetadata().getEnsembleAt(0).contains(firstBookie),
                     "first bookie in rack 0 not included in ensemble");
             lh.close();
         }