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 7e4cba5e70b582b1561204d5ca1d124b682048be Author: Yan Zhao <[email protected]> AuthorDate: Wed Oct 18 02:49:00 2023 -0500 Ignore the empty `perRegionPlacement` when RegionAwareEnsemblePlacementPolicy#newEnsemble (#4106) Descriptions of the changes in this PR: If the `perRegionPlacement` available bookies are empty, do not pick a bookie from it. (cherry picked from commit 3221aa30924825cb4c1a5b00fb68dec44712946e) --- .../client/RegionAwareEnsemblePlacementPolicy.java | 7 +-- .../TestRegionAwareEnsemblePlacementPolicy.java | 50 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java index b7a7f0e48e..261911a877 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java @@ -323,10 +323,11 @@ public class RegionAwareEnsemblePlacementPolicy extends RackawareEnsemblePlaceme excludedBookies); Set<Node> excludeNodes = convertBookiesToNodes(comprehensiveExclusionBookiesSet); List<String> availableRegions = new ArrayList<>(); - for (String region: perRegionPlacement.keySet()) { - if ((null == disallowBookiePlacementInRegionFeatureName) + for (Map.Entry<String, TopologyAwareEnsemblePlacementPolicy> entry : perRegionPlacement.entrySet()) { + String region = entry.getKey(); + if (!entry.getValue().knownBookies.isEmpty() && (null == disallowBookiePlacementInRegionFeatureName || !featureProvider.scope(region).getFeature(disallowBookiePlacementInRegionFeatureName) - .isAvailable()) { + .isAvailable())) { availableRegions.add(region); } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java index ba9c4f862f..9d8e36a350 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRegionAwareEnsemblePlacementPolicy.java @@ -27,10 +27,15 @@ import static org.apache.bookkeeper.client.RegionAwareEnsemblePlacementPolicy.RE import static org.apache.bookkeeper.client.RegionAwareEnsemblePlacementPolicy.REPP_REGIONS_TO_WRITE; import static org.apache.bookkeeper.client.RoundRobinDistributionSchedule.writeSetFromValues; import static org.apache.bookkeeper.feature.SettableFeatureProvider.DISABLE_ALL; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import com.google.common.collect.Sets; import com.google.common.util.concurrent.ThreadFactoryBuilder; import io.netty.util.HashedWheelTimer; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.net.InetAddress; import java.util.ArrayList; import java.util.HashMap; @@ -499,6 +504,51 @@ public class TestRegionAwareEnsemblePlacementPolicy extends TestCase { } } + @Test + public void testNewEnsembleBookieWithOneEmptyRegion() 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); + // update dns mapping + StaticDNSResolver.addNodeToRack(addr1.getHostName(), NetworkTopology.DEFAULT_REGION_AND_RACK); + StaticDNSResolver.addNodeToRack(addr2.getHostName(), "/region2/r2"); + StaticDNSResolver.addNodeToRack(addr3.getHostName(), "/region3/r3"); + StaticDNSResolver.addNodeToRack(addr4.getHostName(), "/region4/r4"); + // Update cluster + Set<BookieId> addrs = new HashSet<BookieId>(); + addrs.add(addr1.toBookieId()); + addrs.add(addr2.toBookieId()); + addrs.add(addr3.toBookieId()); + addrs.add(addr4.toBookieId()); + + Field logField = repp.getClass().getDeclaredField("LOG"); + Logger mockLogger = mock(Logger.class); + + Field modifiers = Field.class.getDeclaredField("modifiers"); + modifiers.setAccessible(true); + modifiers.setInt(logField, logField.getModifiers() & ~Modifier.FINAL); + logField.setAccessible(true); + logField.set(null, mockLogger); + + repp.onClusterChanged(addrs, new HashSet<BookieId>()); + repp.newEnsemble(3, 3, 3, null, + new HashSet<BookieId>()).getResult(); + verify(mockLogger, times(0)).warn("Could not allocate {} bookies in region {}, try allocating {} bookies", + 1, "UnknownRegion", 0); + addrs = new HashSet<BookieId>(); + addrs.add(addr2.toBookieId()); + addrs.add(addr3.toBookieId()); + addrs.add(addr4.toBookieId()); + repp.onClusterChanged(addrs, new HashSet<BookieId>()); + + repp.newEnsemble(3, 3, 3, null, + new HashSet<BookieId>()).getResult(); + + verify(mockLogger, times(0)).warn("Could not allocate {} bookies in region {}, try allocating {} bookies", + 1, "UnknownRegion", 0); + } + @Test public void testReplaceBookieWithNotEnoughBookies() throws Exception { BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181);
