This is an automated email from the ASF dual-hosted git repository.
lhotari 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 556024dcf9f [fix][broker] Fix some problems in calculate
totalAvailableBookies in method getExcludedBookiesWithIsolationGroups when some
bookies belongs to multiple isolation groups. (#24091)
556024dcf9f is described below
commit 556024dcf9ff34be369e82e54fa6f878550ec876
Author: hanmz <[email protected]>
AuthorDate: Mon Apr 14 22:36:37 2025 +0800
[fix][broker] Fix some problems in calculate totalAvailableBookies in
method getExcludedBookiesWithIsolationGroups when some bookies belongs to
multiple isolation groups. (#24091)
---
.../IsolatedBookieEnsemblePlacementPolicy.java | 15 ++-
.../IsolatedBookieEnsemblePlacementPolicyTest.java | 119 +++++++++++++++++++++
2 files changed, 129 insertions(+), 5 deletions(-)
diff --git
a/pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java
b/pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java
index 62b7ffa1e29..878bbc4d654 100644
---
a/pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java
+++
b/pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java
@@ -206,6 +206,9 @@ public class IsolatedBookieEnsemblePlacementPolicy extends
RackawareEnsemblePlac
return excludedBookies;
}
Set<String> allGroups = allGroupsBookieMapping.keySet();
+ if (allGroups.isEmpty()) {
+ return excludedBookies;
+ }
int totalAvailableBookiesInPrimaryGroup = 0;
Set<String> primaryIsolationGroup = Collections.emptySet();
Set<String> secondaryIsolationGroup = Collections.emptySet();
@@ -222,9 +225,10 @@ public class IsolatedBookieEnsemblePlacementPolicy extends
RackawareEnsemblePlac
}
} else {
for (String groupBookie : bookiesInGroup) {
- totalAvailableBookiesInPrimaryGroup += knownBookies
- .containsKey(BookieId.parse(groupBookie)) ? 1
: 0;
-
primaryGroupBookies.add(BookieId.parse(groupBookie));
+ BookieId bookieId = BookieId.parse(groupBookie);
+ if (primaryGroupBookies.add(bookieId)) {
+ totalAvailableBookiesInPrimaryGroup +=
knownBookies.containsKey(bookieId) ? 1 : 0;
+ }
}
}
}
@@ -256,8 +260,9 @@ public class IsolatedBookieEnsemblePlacementPolicy extends
RackawareEnsemblePlac
Map<String, BookieInfo> bookieGroup =
allGroupsBookieMapping.get(group);
if (bookieGroup != null && !bookieGroup.isEmpty()) {
for (String bookieAddress : bookieGroup.keySet()) {
-
excludedBookies.remove(BookieId.parse(bookieAddress));
- totalAvailableBookiesFromPrimaryAndSecondary
+= 1;
+ if
(excludedBookies.remove(BookieId.parse(bookieAddress))) {
+
totalAvailableBookiesFromPrimaryAndSecondary += 1;
+ }
}
}
}
diff --git
a/pulsar-broker-common/src/test/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicyTest.java
b/pulsar-broker-common/src/test/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicyTest.java
index beb00197e4e..2c728120980 100644
---
a/pulsar-broker-common/src/test/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicyTest.java
+++
b/pulsar-broker-common/src/test/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicyTest.java
@@ -699,4 +699,123 @@ public class IsolatedBookieEnsemblePlacementPolicyTest {
.newEnsemble(2, 2, 2, Collections.emptyMap(), new
HashSet<>()).getResult();
assertEquals(BookieIdGroup1.containsAll(defaultBookieList),true);
}
+
+ @Test
+ public void testGetExcludedBookiesWithIsolationGroups() throws Exception {
+ Map<String, Map<String, BookieInfo>> bookieMapping = new HashMap<>();
+ final String isolationGroup1 = "Group1";
+ final String isolationGroup2 = "Group2";
+ final String isolationGroup3 = "Group3";
+
+ Map<String, BookieInfo> group1 = new HashMap<>();
+ group1.put(BOOKIE1, BookieInfo.builder().rack("rack0").build());
+ group1.put(BOOKIE2, BookieInfo.builder().rack("rack0").build());
+
+ Map<String, BookieInfo> group2 = new HashMap<>();
+ group2.put(BOOKIE3, BookieInfo.builder().rack("rack1").build());
+ group2.put(BOOKIE4, BookieInfo.builder().rack("rack1").build());
+
+ bookieMapping.put(isolationGroup1, group1);
+ bookieMapping.put(isolationGroup2, group2);
+
+ store.put(BookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,
jsonMapper.writeValueAsBytes(bookieMapping),
+ Optional.empty()).join();
+
+ IsolatedBookieEnsemblePlacementPolicy isolationPolicy = new
IsolatedBookieEnsemblePlacementPolicy();
+ ClientConfiguration bkClientConf = new ClientConfiguration();
+
bkClientConf.setProperty(BookieRackAffinityMapping.METADATA_STORE_INSTANCE,
store);
+
bkClientConf.setProperty(IsolatedBookieEnsemblePlacementPolicy.ISOLATION_BOOKIE_GROUPS,
isolationGroup1);
+
bkClientConf.setProperty(IsolatedBookieEnsemblePlacementPolicy.SECONDARY_ISOLATION_BOOKIE_GROUPS,
isolationGroup2);
+ isolationPolicy.initialize(bkClientConf, Optional.empty(), timer,
SettableFeatureProvider.DISABLE_ALL,
+ NullStatsLogger.INSTANCE,
BookieSocketAddress.LEGACY_BOOKIEID_RESOLVER);
+ isolationPolicy.onClusterChanged(writableBookies, readOnlyBookies);
+
+ /* Test common cases */
+ MutablePair<Set<String>, Set<String>> groups = new MutablePair<>();
+ groups.setLeft(Sets.newHashSet(isolationGroup1));
+ groups.setRight(Sets.newHashSet(""));
+ Set<BookieId> blacklist =
isolationPolicy.getExcludedBookiesWithIsolationGroups(2, groups);
+ assertEquals(blacklist.size(), 2);
+
+ groups.setLeft(Sets.newHashSet(isolationGroup1));
+ groups.setRight(Sets.newHashSet(isolationGroup2));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(2,
groups);
+ assertEquals(blacklist.size(), 2);
+
+ groups.setLeft(Sets.newHashSet(isolationGroup1));
+ groups.setRight(Sets.newHashSet(isolationGroup2));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(3,
groups);
+ assertTrue(blacklist.isEmpty());
+
+ /* Test a bookie belongs to multiple isolation groups */
+ group1 = new HashMap<>();
+ group1.put(BOOKIE1, BookieInfo.builder().rack("rack0").build());
+
+ group2 = new HashMap<>();
+ group2.put(BOOKIE2, BookieInfo.builder().rack("rack0").build());
+ group2.put(BOOKIE3, BookieInfo.builder().rack("rack1").build());
+ group2.put(BOOKIE4, BookieInfo.builder().rack("rack1").build());
+
+ Map<String, BookieInfo> group3 = new HashMap<>();
+ group3.put(BOOKIE1, BookieInfo.builder().rack("rack0").build());
+
+ bookieMapping.put(isolationGroup1, group1);
+ bookieMapping.put(isolationGroup2, group2);
+ bookieMapping.put(isolationGroup3, group3);
+
+ store.put(BookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,
jsonMapper.writeValueAsBytes(bookieMapping),
+ Optional.empty()).join();
+
+ groups.setLeft(Sets.newHashSet(isolationGroup1, isolationGroup3));
+ groups.setRight(Sets.newHashSet(""));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(2,
groups);
+ assertEquals(blacklist.size(), 3);
+
+ /* Test a bookie belongs to multiple isolation groups and
totalAvailableBookiesInPrimaryGroup < ensembleSize */
+ groups.setLeft(Sets.newHashSet(isolationGroup1, isolationGroup3));
+ groups.setRight(Sets.newHashSet(isolationGroup2));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(2,
groups);
+ assertTrue(blacklist.isEmpty());
+
+ /* Test some bookies not set rack config */
+ group1 = new HashMap<>();
+ group1.put(BOOKIE1, BookieInfo.builder().rack("rack0").build());
+
+ group2 = new HashMap<>();
+ group2.put(BOOKIE2, BookieInfo.builder().rack("rack0").build());
+
+ bookieMapping.put(isolationGroup1, group1);
+ bookieMapping.put(isolationGroup2, group2);
+
+ store.put(BookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,
jsonMapper.writeValueAsBytes(bookieMapping),
+ Optional.empty()).join();
+
+ groups.setLeft(Sets.newHashSet(isolationGroup1));
+ groups.setRight(Sets.newHashSet(isolationGroup2));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(2,
groups);
+ assertEquals(blacklist.size(), 2);
+
+ groups.setLeft(Sets.newHashSet(isolationGroup1));
+ groups.setRight(Sets.newHashSet(isolationGroup2));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(3,
groups);
+ assertTrue(blacklist.isEmpty());
+
+ /* Test some bookies not set rack config and
totalAvailableBookiesFromPrimaryAndSecondary < ensembleSize */
+ group1 = new HashMap<>();
+ group1.put(BOOKIE1, BookieInfo.builder().rack("rack0").build());
+
+ group2 = new HashMap<>();
+ group2.put(BOOKIE1, BookieInfo.builder().rack("rack0").build());
+
+ bookieMapping.put(isolationGroup1, group1);
+ bookieMapping.put(isolationGroup2, group2);
+
+ store.put(BookieRackAffinityMapping.BOOKIE_INFO_ROOT_PATH,
jsonMapper.writeValueAsBytes(bookieMapping),
+ Optional.empty()).join();
+
+ groups.setLeft(Sets.newHashSet(isolationGroup1));
+ groups.setRight(Sets.newHashSet(isolationGroup2));
+ blacklist = isolationPolicy.getExcludedBookiesWithIsolationGroups(2,
groups);
+ assertTrue(blacklist.isEmpty());
+ }
}
