You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by ni...@apache.org on 2022/04/06 15:58:52 UTC
[bookkeeper] branch master updated: only update topology when bookie rack changed
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 <ga...@qq.com>
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 <zh...@gmail.com>, Nicolò Boschi <bo...@gmail.com>, Andrey Yegorov <None>, ZhangJian He <sh...@gmail.com>, Enrico Olivelli <eo...@gmail.com>
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;