horizonzy commented on code in PR #4009:
URL: https://github.com/apache/bookkeeper/pull/4009#discussion_r1250430497
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java:
##########
@@ -636,6 +638,75 @@ public void testIsEnsembleAdheringToPlacementPolicy()
throws Exception {
repp.isEnsembleAdheringToPlacementPolicy(ensemble, 3, 3));
}
+ @Test
+ public void testReplaceBookieWithNewEnsemble() throws Exception {
+ BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.1", 3181);
+ BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.2", 3181);
+ BookieSocketAddress addr3 = new BookieSocketAddress("127.0.0.3", 3181);
+ BookieSocketAddress addr4 = new BookieSocketAddress("127.0.0.4", 3181);
+ BookieSocketAddress addr5 = new BookieSocketAddress("127.0.0.5", 3181);
+ // update dns mapping
+ StaticDNSResolver.addNodeToRack(addr1.getHostName(),
"/default-region/r1");
+ StaticDNSResolver.addNodeToRack(addr2.getHostName(),
"/default-region/r1");
+ StaticDNSResolver.addNodeToRack(addr3.getHostName(),
"/default-region/r1");
+ StaticDNSResolver.addNodeToRack(addr4.getHostName(),
"/default-region/r2");
+ StaticDNSResolver.addNodeToRack(addr5.getHostName(),
"/default-region/r3");
+
+ ClientConfiguration conf = (ClientConfiguration) this.conf.clone();
+ conf.setMinNumRacksPerWriteQuorum(3);
+
+ repp = new RackawareEnsemblePlacementPolicy();
+ repp.initialize(conf, Optional.<DNSToSwitchMapping> empty(), timer,
DISABLE_ALL,
+ NullStatsLogger.INSTANCE,
BookieSocketAddress.LEGACY_BOOKIEID_RESOLVER);
+ repp.withDefaultRack(NetworkTopology.DEFAULT_REGION_AND_RACK);
+
+ // Update cluster
+ Set<BookieId> addrs = new HashSet<BookieId>();
+ addrs.add(addr1.toBookieId());
+ addrs.add(addr2.toBookieId());
+ addrs.add(addr3.toBookieId());
+ addrs.add(addr4.toBookieId());
+ addrs.add(addr5.toBookieId());
+ repp.onClusterChanged(addrs, new HashSet<>());
+
+ Set<Integer> bookieIndexesToRereplicate = new HashSet<>();
+ bookieIndexesToRereplicate.add(1);
+ bookieIndexesToRereplicate.add(2);
+
+ List<BookieId> ensemble = new ArrayList<>();
+ ensemble.add(addr1.toBookieId());
+ ensemble.add(addr2.toBookieId());
+ ensemble.add(addr3.toBookieId());
+
+ Set<BookieId> bookiesToExclude = Sets.newHashSet();
+
+ for (Integer bookieIndex : bookieIndexesToRereplicate) {
+ BookieId bookie = ensemble.get(bookieIndex);
+ bookiesToExclude.add(bookie);
+ }
+
+ List<BookieId> newEnsemble = new ArrayList<>(ensemble);
+
+ int i = 0;
+ for (Integer bookieIndex : bookieIndexesToRereplicate) {
Review Comment:
For getReplacementBookiesByIndexes, this change doesn't influence the
result.
https://github.com/apache/bookkeeper/blob/52e780f326c6cb5f303b51aa6ab5363c8e94b074/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java#L1088-L1110
Line_1109 already add the new bookie into `bookiesToExclude`.
It just didn't update the new bookie to the ensemble. It's different to test
for `getReplacementBookiesByIndexes `, so I add a test to mock the internal
logic in the `getReplacementBookiesByIndexes`.
--
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]
