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.
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 5fcfd0f..19729a4 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
@@ -324,10 +324,11 @@
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 ba9c4f8..9d8e36a 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.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;
@@ -500,6 +505,51 @@
}
@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);
BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.3", 3181);
