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_r286271121
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
##########
@@ -956,4 +958,121 @@ public void testLedgerDeletionIdempotency() throws
Exception {
bk.deleteLedger(ledgerId);
bk.close();
}
+
+ /**
+ * Mock of RackawareEnsemblePlacementPolicy. Overrides
areAckedBookiesAdheringToPlacementPolicy to only return true
+ * when ackedBookies consists of 3 bookies. Also adds a counter to track
the number of invocations of
+ * areAckedBookiesAdheringToPlacementPolicy and getter/setter for that.
+ */
+ public static class MockRackawareEnsemblePlacementPolicy extends
RackawareEnsemblePlacementPolicy {
+ private AtomicInteger counter = new AtomicInteger(0);
+ private int writeQuorumSizeToUseForTesting = 3;
+
+ public int getCounter() {
+ return counter.get();
+ }
+
+ void resetCounter() {
+ this.counter.set(0);
+ }
+
+ @Override
+ public boolean
areAckedBookiesAdheringToPlacementPolicy(Set<BookieSocketAddress> ackedBookies)
{
+ counter.incrementAndGet();
+ return ackedBookies.size() == writeQuorumSizeToUseForTesting;
+ }
+ }
+
+ /**
+ * Test to verify that PendingAddOp waits for success condition from
areAckedBookiesAdheringToPlacementPolicy
+ * before returning success to client.
+ * The test :
+ * 1. Sets `enforceMinNumFaultDomainsForWrite` to true ,
`minNumRacksPerWriteQuorum` to 2 and the placement policy
+ * to the mocked MockRackawareEnsemblePlacementPolicy.
+ * 2. Resets the counter counting how many times was
enforceMinNumFaultDomainsForWrite not met before succeeding.
+ * 3. Creates a ledger using EnsembleSize=WriteQuorumSize=3 and
ackQuorumSize=2.
+ * 4. Picks a bookie from the current ensemble to be put to sleep before
attempting write.
+ * 5. Puts the picked bookie to sleep and attempts write.
+ * 6. Verifies that the write has not succeeded since the overriden
areAckedBookiesAdheringToPlacementPolicy is not
+ * met.
+ * 7. Awakens the sleeping bookie and check to make sure that the write
succeeds.
+ * 8. Verifies that the number of times, ackQuorum was met but
areAckedBookiesAdheringToPlacementPolicy was not met
+ * is 2(value received from the counter in the placement policy). This
should be 2 because it would be called
+ * once when 2 bookies have responded but the third one is still
sleeping, and once when the bookie is woken up.
+ *
+ * @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.setEnsemblePlacementPolicy(MockRackawareEnsemblePlacementPolicy.class);
+
+ conf.setMinNumRacksPerWriteQuorum(2);
+ conf.setEnforceMinNumFaultDomainsForWrite(true);
+
+ // Abnormal values for testing to prevent timeouts
+ conf.setAddEntryTimeout(300);
+ BookKeeperTestClient bk = new BookKeeperTestClient(conf);
+
+ MockRackawareEnsemblePlacementPolicy currPlacementPolicy =
+ (MockRackawareEnsemblePlacementPolicy) bk.getPlacementPolicy();
+ currPlacementPolicy.resetCounter();
+ BookieSocketAddress bookieToSleep;
+
+ try (LedgerHandle lh = bk.createLedger(3, 3, 2, digestType, password))
{
+ CountDownLatch sleepLatch = new CountDownLatch(1);
+
+ Thread bookieSleeperCountdown = new Thread(() -> {
+ try {
+ LOG.info("Counting down 10 seconds before waking sleeping
bookie");
+ sleepLatch.await(10, TimeUnit.SECONDS);
+ LOG.info("Picked bookie awake");
+ } catch (InterruptedException ignored) {
+ }
+
+ sleepLatch.countDown();
+ });
+
+ Thread writeToLedger = new Thread(() -> {
+ try {
+ LOG.info("Initiating write for entry");
+ long entryId = lh.addEntry(data);
+ LOG.info("Wrote entry with entryId = {}", entryId);
+ assertTrue(entryId >= 0);
+ } catch (InterruptedException | BKException ignored) {
+ // Fail the test if the write times out
+ fail("Write should not have thrown error");
+ }
+ });
+
+ bookieToSleep = lh.getCurrentEnsemble().get(0);
+
+ // Putting non default rack bookie to sleep
Review comment:
Doing.
----------------------------------------------------------------
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
