This is an automated email from the ASF dual-hosted git repository.
nicoloboschi 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 67c963a1b only update topology when bookie rack changed
67c963a1b is described below
commit 67c963a1b02de04166778d906f4cffe89dd86562
Author: gaozhangmin <[email protected]>
AuthorDate: Wed Apr 6 23:58:47 2022 +0800
only update topology when bookie rack changed
It is unnecessary to update topology, removing and adding the same
bookieNode, if the rack of bookie stay unchanged.
Reviewers: Yong Zhang <[email protected]>, Nicolò Boschi
<[email protected]>, Andrey Yegorov <None>, ZhangJian He
<[email protected]>, Enrico Olivelli <[email protected]>
This closes #2790 from gaozhangmin/remove-unnecessary-update-rack
---
.../TopologyAwareEnsemblePlacementPolicy.java | 10 +++--
.../TestRackawareEnsemblePlacementPolicy.java | 49 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TopologyAwareEnsemblePlacementPolicy.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TopologyAwareEnsemblePlacementPolicy.java
index 7fdf3c5dd..a1796efea 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TopologyAwareEnsemblePlacementPolicy.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TopologyAwareEnsemblePlacementPolicy.java
@@ -749,10 +749,12 @@ abstract class TopologyAwareEnsemblePlacementPolicy
implements
if (node != null) {
// refresh the rack info if its a known bookie
BookieNode newNode = createBookieNode(bookieAddress);
- topology.remove(node);
- topology.add(newNode);
- knownBookies.put(bookieAddress, newNode);
- historyBookies.put(bookieAddress, newNode);
+ if
(!newNode.getNetworkLocation().equals(node.getNetworkLocation())) {
+ topology.remove(node);
+ topology.add(newNode);
+ knownBookies.put(bookieAddress, newNode);
+ historyBookies.put(bookieAddress, newNode);
+ }
}
} catch (IllegalArgumentException |
NetworkTopologyImpl.InvalidTopologyException e) {
LOG.error("Failed to update bookie rack info: {} ",
bookieAddress, e);
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 28a27ee61..568fdfc2f 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
@@ -2142,6 +2142,55 @@ public class TestRackawareEnsemblePlacementPolicy
extends TestCase {
assertTrue(shuffleOccurred);
}
+ @Test
+ public void testUpdateTopologyWithRackChange() throws Exception {
+ String defaultRackForThisTest =
NetworkTopology.DEFAULT_REGION_AND_RACK;
+ repp.uninitalize();
+ updateMyRack(defaultRackForThisTest);
+
+ // Update cluster
+ BookieSocketAddress newAddr1 = new BookieSocketAddress("127.0.0.100",
3181);
+ BookieSocketAddress newAddr2 = new BookieSocketAddress("127.0.0.101",
3181);
+ BookieSocketAddress newAddr3 = new BookieSocketAddress("127.0.0.102",
3181);
+ BookieSocketAddress newAddr4 = new BookieSocketAddress("127.0.0.103",
3181);
+
+ // update dns mapping
+ StaticDNSResolver.addNodeToRack(newAddr1.getHostName(),
defaultRackForThisTest);
+ StaticDNSResolver.addNodeToRack(newAddr2.getHostName(),
defaultRackForThisTest);
+ StaticDNSResolver.addNodeToRack(newAddr3.getHostName(),
defaultRackForThisTest);
+ StaticDNSResolver.addNodeToRack(newAddr4.getHostName(),
defaultRackForThisTest);
+
+ TestStatsProvider statsProvider = new TestStatsProvider();
+ TestStatsLogger statsLogger = statsProvider.getStatsLogger("");
+
+ repp = new RackawareEnsemblePlacementPolicy();
+ repp.initialize(conf, Optional.<DNSToSwitchMapping> empty(), timer,
+ DISABLE_ALL, statsLogger,
BookieSocketAddress.LEGACY_BOOKIEID_RESOLVER);
+ repp.withDefaultRack(defaultRackForThisTest);
+
+ Gauge<? extends Number> numBookiesInDefaultRackGauge = statsLogger
+
.getGauge(BookKeeperClientStats.NUM_WRITABLE_BOOKIES_IN_DEFAULT_RACK);
+
+ Set<BookieId> writeableBookies = new HashSet<>();
+ Set<BookieId> readOnlyBookies = new HashSet<>();
+ writeableBookies.add(newAddr1.toBookieId());
+ writeableBookies.add(newAddr2.toBookieId());
+ writeableBookies.add(newAddr3.toBookieId());
+ writeableBookies.add(newAddr4.toBookieId());
+ repp.onClusterChanged(writeableBookies, readOnlyBookies);
+ // only writable bookie - newAddr1 in default rack
+ assertEquals("NUM_WRITABLE_BOOKIES_IN_DEFAULT_RACK guage value", 4,
numBookiesInDefaultRackGauge.getSample());
+
+ // newAddr4 rack is changed and it is not in default anymore
+ StaticDNSResolver
+ .changeRack(Collections.singletonList(newAddr3),
Collections.singletonList("/default-region/r4"));
+ assertEquals("NUM_WRITABLE_BOOKIES_IN_DEFAULT_RACK guage value", 3,
numBookiesInDefaultRackGauge.getSample());
+
+ StaticDNSResolver
+ .changeRack(Collections.singletonList(newAddr1),
Collections.singletonList(defaultRackForThisTest));
+ assertEquals("NUM_WRITABLE_BOOKIES_IN_DEFAULT_RACK guage value", 3,
numBookiesInDefaultRackGauge.getSample());
+ }
+
@Test
public void testNumBookiesInDefaultRackGauge() throws Exception {
String defaultRackForThisTest =
NetworkTopology.DEFAULT_REGION_AND_RACK;