This is an automated email from the ASF dual-hosted git repository. yong pushed a commit to branch branch-4.15 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 4a0b72ac42241d9dd5258db9f997c38490dff808 Author: Yan Zhao <[email protected]> AuthorDate: Tue Jul 26 09:17:26 2022 +0800 If ensembleList is empty, return PlacementPolicyAdherence.FAIL. (#3369) Descriptions of the changes in this PR: If the ensembleList is empty, should not return PlacementPolicyAdherence.MEETS_STRICT. Return PlacementPolicyAdherence.FAIL to remind user. (cherry picked from commit 590de2b0e2f39466f5c466356b8106ed736dc303) --- .../RackawareEnsemblePlacementPolicyImpl.java | 3 ++ .../ZoneawareEnsemblePlacementPolicyImpl.java | 4 ++ .../TestRackawareEnsemblePlacementPolicy.java | 48 ++++++++++++++++++++++ .../TestZoneawareEnsemblePlacementPolicy.java | 4 ++ 4 files changed, 59 insertions(+) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java index 1647deb666..5185dc4753 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java @@ -1017,6 +1017,9 @@ public class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsembleP @Override public PlacementPolicyAdherence isEnsembleAdheringToPlacementPolicy(List<BookieId> ensembleList, int writeQuorumSize, int ackQuorumSize) { + if (CollectionUtils.isEmpty(ensembleList)) { + return PlacementPolicyAdherence.FAIL; + } int ensembleSize = ensembleList.size(); int minNumRacksPerWriteQuorumForThisEnsemble = Math.min(writeQuorumSize, minNumRacksPerWriteQuorum); HashSet<String> racksInQuorum = new HashSet<String>(); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ZoneawareEnsemblePlacementPolicyImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ZoneawareEnsemblePlacementPolicyImpl.java index 61b81ed303..bb0c11bee1 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ZoneawareEnsemblePlacementPolicyImpl.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ZoneawareEnsemblePlacementPolicyImpl.java @@ -66,6 +66,7 @@ import org.apache.bookkeeper.stats.Counter; import org.apache.bookkeeper.stats.Gauge; import org.apache.bookkeeper.stats.StatsLogger; import org.apache.bookkeeper.stats.annotations.StatsDoc; +import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -863,6 +864,9 @@ public class ZoneawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsembleP @Override public PlacementPolicyAdherence isEnsembleAdheringToPlacementPolicy(List<BookieId> ensembleList, int writeQuorumSize, int ackQuorumSize) { + if (CollectionUtils.isEmpty(ensembleList)) { + return PlacementPolicyAdherence.FAIL; + } PlacementPolicyAdherence placementPolicyAdherence = PlacementPolicyAdherence.MEETS_STRICT; rwLock.readLock().lock(); try { diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java index 568fdfc2f2..0e9233136e 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java @@ -580,6 +580,54 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase { assertEquals("writeSet should be in original order", origWriteSet, reorderSet); } + @Test + public void testIsEnsembleAdheringToPlacementPolicy() throws Exception { + BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); + BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181); + BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.4", 3181); + BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.5", 3181); + BookieSocketAddress addr5 = new BookieSocketAddress("127.0.0.6", 3181); + BookieSocketAddress addr6 = new BookieSocketAddress("127.0.0.7", 3181); + // update dns mapping + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); + StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/default-region/r2"); + StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/default-region/r2"); + StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/default-region/r3"); + StaticDNSResolver.addNodeToRack(addr5.getHostName(), "/default-region/r3"); + StaticDNSResolver.addNodeToRack(addr6.getHostName(), "/default-region/r3"); + // Update cluster + Set<BookieId> addrs = new HashSet<BookieId>(); + addrs.add(addr1.toBookieId()); + addrs.add(addr2.toBookieId()); + addrs.add(addr3.toBookieId()); + addrs.add(addr4.toBookieId()); + repp.onClusterChanged(addrs, new HashSet<BookieId>()); + + List<BookieId> emptyEnsemble = new ArrayList<>(); + assertEquals("PlacementPolicyAdherence", PlacementPolicyAdherence.FAIL, + repp.isEnsembleAdheringToPlacementPolicy(emptyEnsemble, 3, 3)); + + List<BookieId> ensemble = new ArrayList<>(); + ensemble.add(addr1.toBookieId()); + ensemble.add(addr2.toBookieId()); + assertEquals("PlacementPolicyAdherence", PlacementPolicyAdherence.MEETS_STRICT, + repp.isEnsembleAdheringToPlacementPolicy(ensemble, 3, 3)); + + ensemble = new ArrayList<>(); + ensemble.add(addr1.toBookieId()); + ensemble.add(addr2.toBookieId()); + ensemble.add(addr3.toBookieId()); + assertEquals("PlacementPolicyAdherence", PlacementPolicyAdherence.MEETS_STRICT, + repp.isEnsembleAdheringToPlacementPolicy(ensemble, 3, 3)); + + ensemble = new ArrayList<>(); + ensemble.add(addr4.toBookieId()); + ensemble.add(addr5.toBookieId()); + ensemble.add(addr6.toBookieId()); + assertEquals("PlacementPolicyAdherence", PlacementPolicyAdherence.FAIL, + repp.isEnsembleAdheringToPlacementPolicy(ensemble, 3, 3)); + } + @Test public void testReplaceBookieWithEnoughBookiesInSameRack() throws Exception { BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestZoneawareEnsemblePlacementPolicy.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestZoneawareEnsemblePlacementPolicy.java index 12c0674249..ccfa73453c 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestZoneawareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestZoneawareEnsemblePlacementPolicy.java @@ -1263,6 +1263,10 @@ public class TestZoneawareEnsemblePlacementPolicy extends TestCase { NullStatsLogger.INSTANCE, BookieSocketAddress.LEGACY_BOOKIEID_RESOLVER); zepp.withDefaultFaultDomain(NetworkTopology.DEFAULT_ZONE_AND_UPGRADEDOMAIN); + List<BookieId> emptyEnsmeble = new ArrayList<>(); + assertEquals("PlacementPolicyAdherence", PlacementPolicyAdherence.FAIL, + zepp.isEnsembleAdheringToPlacementPolicy(emptyEnsmeble, 3, 2)); + List<BookieId> ensemble = new ArrayList<BookieId>(); ensemble.add(addr1.toBookieId()); ensemble.add(addr2.toBookieId());
