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_r284552153
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
##########
@@ -956,4 +960,98 @@ public void testLedgerDeletionIdempotency() throws
Exception {
bk.deleteLedger(ledgerId);
bk.close();
}
+
+
+ /**
+ * Test to test the working of enforceMinNumFaultDomainsForWrite
configuration. The test:
+ * 1. Sets up the config to use a minimum of 2 racks per write quorum
+ * 2. Enables both enforceMinNumRacksPerWriteQuorum and
enforceMinNumFaultDomainsForWrite.
+ * 3. Starts up 4 bookies in rack 0 (`/default-region/rack0`) and 1 in
rack 1 `/default-region/rack1`
+ * 4. Creates a ledger using ensembleSize=wqSize=3.
+ * 5. The rack 1 bookie is put to sleep
+ * 6. AddEntry is attempted.
+ * 7. A separate thread countdowns 10 seconds and then awakens the
sleeping bookie.
+ * 8. Success of addEntry is checked.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testEnforceMinNumFaultDomainsForWrite() throws Exception {
+ byte[] data = "foobar".getBytes();
+ byte[] password = "testPasswd".getBytes();
+
+ ClientConfiguration conf = new ClientConfiguration();
+ conf.setMetadataServiceUri(zkUtil.getMetadataServiceUri());
+ conf.setProperty(REPP_DNS_RESOLVER_CLASS,
StaticDNSResolver.class.getName());
+
conf.setEnsemblePlacementPolicy(RackawareEnsemblePlacementPolicy.class);
+
+ conf.setMinNumRacksPerWriteQuorum(2);
+ conf.setEnforceMinNumRacksPerWriteQuorum(true);
+ conf.setEnforceMinNumFaultDomainsForWrite(true);
+
+ // Abnormal values for testing to prevent timeouts
+ conf.setAddEntryTimeout(300);
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(conf.toString());
+ }
+
+ // Assign all initial bookies started to the default rack by modifying
the `localhost` in the StaticDNSResolver
+ StaticDNSResolver.reset();
+ StaticDNSResolver.addNodeToRack("localhost",
NetworkTopology.DEFAULT_REGION + "/rack0");
+
+ try (BookKeeperTestClient bk = new BookKeeperTestClient(conf)) {
+ // Modify localhost in StaticDNSResolver to assign next started
bookie to a different rack("/rack1")
+ StaticDNSResolver.addNodeToRack("localhost",
NetworkTopology.DEFAULT_REGION + "/rack1");
Review comment:
@ankit-j as we discussed this test logic would not work, since
DNSToSwitchMapping.resolve maps DNS-name/IP-address to rack but it does not map
BookieSocketAddress/bookieid (which includes bookie server port number) to
rack. So in this test, since all the bookies are running in localhost (with
different portnumbers), technically all bookies would be part of same rack.
To test this feature you can do the following
1) validating 'areAckedBookiesAdheringToPlacementPolicy' method logic. For
this you can do something similar to what i did for testing
'isEnsembleAdheringToPlacementPolicy' in
https://github.com/apache/bookkeeper/commit/42fe1506afe04b8068b897c530c7ca7188da503e
commit
2) validating PendingAddOp logic at Bookkeeper client API level. Lets do
Dependency Injection testing for this. For this create
MockRackawareEnsemblePlacementPolicy, which extends
RackawareEnsemblePlacementPolicy and in MockRackawareEnsemblePlacementPolicy
override areAckedBookiesAdheringToPlacementPolicy method. In this method add
your conditions/logic as you wish to test the case that PendingAddOp doesn't
consider itself complete just by receiving ack number of responses but instead
it would wait untill areAckedBookiesAdheringToPlacementPolicy returns true.
----------------------------------------------------------------
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
