reddycharan commented on a change in pull request #2096: Entries must be
acknowledged by bookies in multiple fault domains before being acknowledged to
client
URL: https://github.com/apache/bookkeeper/pull/2096#discussion_r286255569
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java
##########
@@ -2233,4 +2233,93 @@ public void
testNewEnsembleExcludesDefaultRackBookiesEnforceMinNumRacks() throws
+ bookiesOfDefaultRackInEnsemble,
bookiesOfDefaultRackInEnsemble.isEmpty());
}
}
+
+ /**
+ * This tests areAckedBookiesAdheringToPlacementPolicy function in
RackawareEnsemblePlacementPolicy.
+ */
+ @Test
+ public void testAreAckedBookiesAdheringToPlacementPolicy() throws
Exception {
+ String defaultRackForThisTest =
NetworkTopology.DEFAULT_REGION_AND_RACK;
+ repp.uninitalize();
+ updateMyRack(defaultRackForThisTest);
+
+ ClientConfiguration conf = new ClientConfiguration(this.conf);
+ conf.setMinNumRacksPerWriteQuorum(2);
+ conf.setEnforceMinNumRacksPerWriteQuorum(true);
+ conf.setEnforceMinNumFaultDomainsForWrite(true);
+
+ TestStatsProvider statsProvider = new TestStatsProvider();
+ TestStatsLogger statsLogger = statsProvider.getStatsLogger("");
+
+ repp = new RackawareEnsemblePlacementPolicy();
+ repp.initialize(conf, Optional.<DNSToSwitchMapping> empty(), timer,
DISABLE_ALL, statsLogger);
+ repp.withDefaultRack(defaultRackForThisTest);
+
+ int ensembleSize = 7;
+ int writeQuorumSize = 3;
+ int ackQuorumSize = 2;
+
+ int numOfBookiesInDefaultRack = 7;
+ int numOfRacks = 3;
+ int numOfBookiesPerRack = 3;
+
+ List<BookieSocketAddress> bookieSocketAddressesDefaultRack = new
ArrayList<>();
+ List<BookieSocketAddress> bookieSocketAddressesNonDefaultRack = new
ArrayList<>();
+ Set<BookieSocketAddress> writableBookies;
+ Set<BookieSocketAddress> bookiesForEntry = new HashSet<>();
+
+ for (int i = 0; i < numOfRacks; i++) {
+ for (int j = 0; j < numOfBookiesPerRack; j++) {
+ int index = i * numOfBookiesPerRack + j;
+ bookieSocketAddressesNonDefaultRack.add(new
BookieSocketAddress("128.0.0." + index, 3181));
+
StaticDNSResolver.addNodeToRack(bookieSocketAddressesNonDefaultRack.get(index).getHostName(),
+ "/default-region/r" + i);
+ }
+ }
+
+ for (int i = 0; i < numOfBookiesInDefaultRack; i++) {
+ bookieSocketAddressesDefaultRack.add(new
BookieSocketAddress("127.0.0." + (i + 100), 3181));
+
StaticDNSResolver.addNodeToRack(bookieSocketAddressesDefaultRack.get(i).getHostName(),
+ defaultRackForThisTest);
+ }
+
+ writableBookies = new HashSet<>(bookieSocketAddressesNonDefaultRack);
+ writableBookies.addAll(bookieSocketAddressesDefaultRack);
+ repp.onClusterChanged(writableBookies, new HashSet<>());
+
+ // Case 1 : Bookies in the ensemble from the same rack.
+ // Manually crafting the ensemble here to create the error case when
the check should return false
+
+ List<BookieSocketAddress> ensemble = new
ArrayList<>(bookieSocketAddressesDefaultRack);
+ for (int entryId = 0; entryId < 10; entryId++) {
+ DistributionSchedule ds = new
RoundRobinDistributionSchedule(writeQuorumSize, ackQuorumSize, ensembleSize);
+ DistributionSchedule.WriteSet ws = ds.getWriteSet(entryId);
+
+ for (int i = 0; i < ws.size(); i++) {
+ bookiesForEntry.add(ensemble.get(ws.get(i)));
+ }
+
+
assertFalse(repp.areAckedBookiesAdheringToPlacementPolicy(bookiesForEntry));
+ }
+
+ // Case 2 : Bookies in the ensemble from the different racks
+
+ EnsemblePlacementPolicy.PlacementResult<List<BookieSocketAddress>>
+ ensembleResponse = repp.newEnsemble(ensembleSize,
+ writeQuorumSize,
+ ackQuorumSize,
+ null,
+ new HashSet<>());
+ ensemble = ensembleResponse.getResult();
+ for (int entryId = 0; entryId < 10; entryId++) {
+ DistributionSchedule ds = new
RoundRobinDistributionSchedule(writeQuorumSize, ackQuorumSize, ensembleSize);
+ DistributionSchedule.WriteSet ws = ds.getWriteSet(entryId);
+
+ for (int i = 0; i < ws.size(); i++) {
+ bookiesForEntry.add(ensemble.get(ws.get(i)));
+ }
+
+
assertTrue(repp.areAckedBookiesAdheringToPlacementPolicy(bookiesForEntry));
Review comment:
the two cases you are validating are kind of extreme cases
assertFalse in line 2302 and assertTrue here. Like all the bookies either
belonging to just one rack and bookies belonging to different cases. How about
a normal case where writequorum of 3 contains bookies from 2 different racks
and calling this method with just bookiesForEntry with 2 bookies of a same rack.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
