This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 3221aa3092 Ignore the empty `perRegionPlacement` when
RegionAwareEnsemblePlacementPolicy#newEnsemble (#4106)
3221aa3092 is described below
commit 3221aa30924825cb4c1a5b00fb68dec44712946e
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.
---
.../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 5fcfd0f94f..19729a4bde 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 @@ 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);
