You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2019/05/21 23:13:17 UTC
[GitHub] [bookkeeper] 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
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:
users@infra.apache.org
With regards,
Apache Git Services