[pulsar] branch master updated: [pulsar-broker] zk-isolation placement policy should be able to identify overlapped bookies (#3919)

[mm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mmerli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new cf48435  [pulsar-broker] zk-isolation placement policy should be able to identify overlapped bookies (#3919)
cf48435 is described below

commit cf48435e0170768bd42278c2609694c8498d46a7
Author: Rajan Dhabalia <rd...@apache.org>
AuthorDate: Sat Mar 30 07:37:01 2019 -0700

    [pulsar-broker] zk-isolation placement policy should be able to identify overlapped bookies (#3919)
---
 .../ZkIsolatedBookieEnsemblePlacementPolicy.java   | 13 ++++++
 ...kIsolatedBookieEnsemblePlacementPolicyTest.java | 54 ++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicy.java b/pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicy.java
index fb8d4a9..5db20bb 100644
--- a/pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicy.java
+++ b/pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicy.java
@@ -36,6 +36,7 @@ import org.apache.bookkeeper.net.DNSToSwitchMapping;
 import org.apache.bookkeeper.stats.StatsLogger;
 import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
 import org.apache.commons.configuration.Configuration;
+import org.apache.pulsar.common.policies.data.BookieInfo;
 import org.apache.pulsar.common.policies.data.BookiesRackConfiguration;
 import org.apache.pulsar.common.util.ObjectMapperFactory;
 import org.apache.pulsar.zookeeper.ZooKeeperCache.Deserializer;
@@ -147,6 +148,18 @@ public class ZkIsolatedBookieEnsemblePlacementPolicy extends RackawareEnsemblePl
                         }
                     }
                 }
+                // sometime while doing isolation, user might not want to remove isolated bookies from other default
+                // groups. so, same set of bookies could be overlapped into isolated-group and other default groups. so,
+                // try to remove those overlapped bookies from excluded-bookie list because ther are also part of
+                // isolated-group bookies.
+                for (String group : isolationGroups) {
+                    Map<String, BookieInfo> bookieGroup = allGroupsBookieMapping.get(group);
+                    if (bookieGroup != null && !bookieGroup.isEmpty()) {
+                        for (String bookieAddress : bookieGroup.keySet()) {
+                            blacklistedBookies.remove(new BookieSocketAddress(bookieAddress));
+                        }
+                    }
+                }
             }
         } catch (Exception e) {
             LOG.warn("Error getting bookie isolation info from zk: {}", e.getMessage());
diff --git a/pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicyTest.java b/pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicyTest.java
index 7ed186d..da0725d 100644
--- a/pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicyTest.java
+++ b/pulsar-zookeeper-utils/src/test/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicyTest.java
@@ -299,4 +299,58 @@ public class ZkIsolatedBookieEnsemblePlacementPolicyTest {
 
         isolationPolicy.newEnsemble(4, 4, 4, Collections.emptyMap(), new HashSet<>());
     }
+    
+    /**
+     * validates overlapped bookies between default-groups and isolated-groups.
+     * 
+     * <pre>
+     * a. default-group has all 5 bookies.
+     * b. 3 of the default-group bookies have been added to isolated-group without being removed from default-group.
+     * c. isolated-policy-placement should be identify those 3 overlapped bookies and exclude them from blacklisted bookies.
+     * </pre>
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testOverlappedBookies() throws Exception {
+        Map<String, Map<String, BookieInfo>> bookieMapping = new HashMap<>();
+        Map<String, BookieInfo> defaultBookieGroup = new HashMap<>();
+        final String isolatedGroup = "isolatedGroup";
+
+        defaultBookieGroup.put(BOOKIE1, new BookieInfo("rack0", null));
+        defaultBookieGroup.put(BOOKIE2, new BookieInfo("rack1", null));
+        defaultBookieGroup.put(BOOKIE3, new BookieInfo("rack1", null));
+        defaultBookieGroup.put(BOOKIE4, new BookieInfo("rack1", null));
+        defaultBookieGroup.put(BOOKIE5, new BookieInfo("rack1", null));
+
+        Map<String, BookieInfo> isolatedBookieGroup = new HashMap<>();
+        isolatedBookieGroup.put(BOOKIE1, new BookieInfo("rack1", null));
+        isolatedBookieGroup.put(BOOKIE2, new BookieInfo("rack0", null));
+        isolatedBookieGroup.put(BOOKIE4, new BookieInfo("rack0", null));
+
+        bookieMapping.put("default", defaultBookieGroup);
+        bookieMapping.put(isolatedGroup, isolatedBookieGroup);
+
+        ZkUtils.createFullPathOptimistic(localZkc, ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,
+                jsonMapper.writeValueAsBytes(bookieMapping), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+
+        Thread.sleep(100);
+
+        ZkIsolatedBookieEnsemblePlacementPolicy isolationPolicy = new ZkIsolatedBookieEnsemblePlacementPolicy();
+        ClientConfiguration bkClientConf = new ClientConfiguration();
+        bkClientConf.setProperty(ZooKeeperCache.ZK_CACHE_INSTANCE, new ZooKeeperCache(localZkc, 30) {
+        });
+        bkClientConf.setProperty(ZkIsolatedBookieEnsemblePlacementPolicy.ISOLATION_BOOKIE_GROUPS, isolatedGroup);
+        isolationPolicy.initialize(bkClientConf, Optional.empty(), timer, SettableFeatureProvider.DISABLE_ALL,
+                NullStatsLogger.INSTANCE);
+        isolationPolicy.onClusterChanged(writableBookies, readOnlyBookies);
+
+        List<BookieSocketAddress> ensemble = isolationPolicy
+                .newEnsemble(3, 3, 2, Collections.emptyMap(), new HashSet<>()).getResult();
+        assertTrue(ensemble.contains(new BookieSocketAddress(BOOKIE1)));
+        assertTrue(ensemble.contains(new BookieSocketAddress(BOOKIE2)));
+        assertTrue(ensemble.contains(new BookieSocketAddress(BOOKIE4)));
+
+        localZkc.delete(ZkBookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH, -1);
+    }
 }