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 2018/05/28 06:16:05 UTC
[GitHub] eolivelli commented on a change in pull request #1436: BP-14
force() API - client side implementation
eolivelli commented on a change in pull request #1436: BP-14 force() API - client side implementation
URL: https://github.com/apache/bookkeeper/pull/1436#discussion_r190806886
##########
File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/client/DeferredSyncTest.java
##########
@@ -49,10 +51,111 @@ public void testAddEntryLastAddConfirmedDoesNotAdvance() throws Exception {
}
long lastEntryID = result(wh.appendAsync(DATA));
assertEquals(NUM_ENTRIES - 1, lastEntryID);
+ assertEquals(NUM_ENTRIES - 1, wh.getLastAddPushed());
+ assertEquals(-1, wh.getLastAddConfirmed());
+ }
+ }
+
+ @Test
+ public void testAddEntryLastAddConfirmedAdvanceWithForce() throws Exception {
+ try (WriteHandle wh = result(newCreateLedgerOp()
+ .withEnsembleSize(3)
+ .withWriteQuorumSize(3)
+ .withAckQuorumSize(2)
+ .withPassword(PASSWORD)
+ .withWriteFlags(WriteFlag.DEFERRED_SYNC)
+ .execute())) {
+ for (int i = 0; i < NUM_ENTRIES - 1; i++) {
+ result(wh.appendAsync(DATA));
+ }
+ long lastEntryID = result(wh.appendAsync(DATA));
+ assertEquals(NUM_ENTRIES - 1, lastEntryID);
+ assertEquals(NUM_ENTRIES - 1, wh.getLastAddPushed());
+ assertEquals(-1, wh.getLastAddConfirmed());
+ result(wh.force());
+ assertEquals(NUM_ENTRIES - 1, wh.getLastAddConfirmed());
+ }
+ }
+
+ @Test
+ public void testForceRequiresFullEnsemble() throws Exception {
+ try (WriteHandle wh = result(newCreateLedgerOp()
+ .withEnsembleSize(3)
+ .withWriteQuorumSize(2)
+ .withAckQuorumSize(2)
+ .withPassword(PASSWORD)
+ .withWriteFlags(WriteFlag.DEFERRED_SYNC)
+ .execute())) {
+ for (int i = 0; i < NUM_ENTRIES - 1; i++) {
+ result(wh.appendAsync(DATA));
+ }
+ long lastEntryID = result(wh.appendAsync(DATA));
+ assertEquals(NUM_ENTRIES - 1, lastEntryID);
+ assertEquals(NUM_ENTRIES - 1, wh.getLastAddPushed());
+ assertEquals(-1, wh.getLastAddConfirmed());
+
+ BookieSocketAddress bookieAddress = wh.getLedgerMetadata().getEnsembleAt(wh.getLastAddPushed()).get(0);
+ killBookie(bookieAddress);
+
+ // write should succeed (we still have 2 bookies out of 3)
+ result(wh.appendAsync(DATA));
+
+ // force cannot go, it must be acknowledged by all of the bookies in the ensamble
+ try {
+ result(wh.force());
+ } catch (BKException.BKBookieException failed) {
+ }
+ // bookie comes up again, force must succeed
+ startKilledBookie(bookieAddress);
+ result(wh.force());
+ }
+ }
+
+ @Test
+ public void testForceWillAdvanceLacOnlyUpToLastAcknoledgedWrite() throws Exception {
+ try (WriteHandle wh = result(newCreateLedgerOp()
+ .withEnsembleSize(3)
+ .withWriteQuorumSize(3)
+ .withAckQuorumSize(3)
+ .withPassword(PASSWORD)
+ .withWriteFlags(WriteFlag.DEFERRED_SYNC)
+ .execute())) {
+ for (int i = 0; i < NUM_ENTRIES - 1; i++) {
+ result(wh.appendAsync(DATA));
+ }
+ long lastEntryIdBeforeSuspend = result(wh.appendAsync(DATA));
+ assertEquals(NUM_ENTRIES - 1, lastEntryIdBeforeSuspend);
+ assertEquals(-1, wh.getLastAddConfirmed());
+
+ // one bookie will stop sending acks
+ BookieSocketAddress bookieAddress = wh.getLedgerMetadata().getEnsembleAt(wh.getLastAddPushed()).get(0);
+ suspendBookieWriteAcks(bookieAddress);
Review comment:
this test does not make really much sense. I will drop it.
This is testing:
- client send *addEntry*
- client send *force*
- client received ack for *force*
- client received ack for *addEntry*
this is not actually possible due to ordered executor
I will add a test with:
- client send *addEntry* for entry 0
- client send *force* at entry 0
- client send *addEntry* for entry 1
- client received ack for *addEntry*
- client received ack for *force* (LAC MUST still be 0, force should 'capture' the last completed addEntry, not the last pushed one)
- client received ack for *addEntry*
I will add similar tests for WriteAdvHandle
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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