ankit-j 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_r286259421
##########
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:
That is not correct. So the ensemble here is picked randomly from 9 bookies
belonging to a total of 3 racks. For a sample run :
```
/default-region/r0/128.0.0.0:3181
/default-region/r0/128.0.0.1:3181
/default-region/r0/128.0.0.2:3181
/default-region/r1/128.0.0.3:3181
/default-region/r1/128.0.0.4:3181
/default-region/r1/128.0.0.5:3181
/default-region/r2/128.0.0.6:3181
/default-region/r2/128.0.0.7:3181
/default-region/r2/128.0.0.8:3181
/default-region/default-rack/127.0.0.100:3181
/default-region/default-rack/127.0.0.101:3181
/default-region/default-rack/127.0.0.102:3181
/default-region/default-rack/127.0.0.103:3181
/default-region/default-rack/127.0.0.104:3181
/default-region/default-rack/127.0.0.105:3181
/default-region/default-rack/127.0.0.106:3181
Ensemble
[127.0.0.104:3181, 128.0.0.8:3181, 127.0.0.106:3181, 128.0.0.3:3181,
128.0.0.0:3181, 128.0.0.6:3181, 128.0.0.1:3181]
[default, r2, default, r1, r0, r2, r0]
```
So the case you mentioned was actually recreated when we tested with entryId
4 which we do
----------------------------------------------------------------
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
