horizonzy commented on code in PR #4057:
URL: https://github.com/apache/bookkeeper/pull/4057#discussion_r1306866548
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java:
##########
@@ -1689,6 +1689,84 @@ public void testNewEnsembleWithPickDifferentRack()
throws Exception {
}
}
+ @Test
+ public void testNewEnsemblePickLocalRackBookiesByHostname() throws
Exception {
Review Comment:
This test cannot cover new features, whether there are new features or not,
the test will pass. I think we should check if the host and rack information of
the localNode align with the expectations.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicy.java:
##########
@@ -57,21 +57,23 @@ protected RackawareEnsemblePlacementPolicy
initialize(DNSToSwitchMapping dnsReso
int
minNumRacksPerWriteQuorum,
boolean
enforceMinNumRacksPerWriteQuorum,
boolean
ignoreLocalNodeInPlacementPolicy,
+ boolean
useHostnameResolveLocalNodePlacementPolicy,
Review Comment:
For compatibility, we can add a new method with the new param
`useHostnameResolveLocalNodePlacementPolicy`. The origin method was just direct
to the new method with the default value `false`.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java:
##########
@@ -1010,6 +1014,22 @@ public boolean getIgnoreLocalNodeInPlacementPolicy() {
return getBoolean(IGNORE_LOCAL_NODE_IN_PLACEMENT_POLICY, false);
}
+ /**
+ * Set the flag to use hostname to resolve local node placement policy.
+ * @param useHostnameResolveLocalNodePlacementPolicy
+ */
+ public void setUseHostnameResolveLocalNodePlacementPolicy(boolean
useHostnameResolveLocalNodePlacementPolicy) {
+ setProperty(USE_HOSTNAME_RESOLVE_LOCAL_NODE_PLACEMENT_POLICY,
useHostnameResolveLocalNodePlacementPolicy);
+ }
+
+ /**
+ * Get whether to use hostname to resolve local node placement policy.
+ * @return
+ */
+ public boolean getUseHostnameResolveLocalNodePlacementPolicy() {
+ return getBoolean(USE_HOSTNAME_RESOLVE_LOCAL_NODE_PLACEMENT_POLICY,
false);
+ }
Review Comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
