Author: ivank Date: Thu Oct 25 14:50:05 2012 New Revision: 1402172 URL: http://svn.apache.org/viewvc?rev=1402172&view=rev Log: BOOKKEEPER-416: LedgerChecker returns underreplicated fragments for an closed ledger with no entries (ivank)
Modified: zookeeper/bookkeeper/trunk/CHANGES.txt zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java Modified: zookeeper/bookkeeper/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1402172&r1=1402171&r2=1402172&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/CHANGES.txt (original) +++ zookeeper/bookkeeper/trunk/CHANGES.txt Thu Oct 25 14:50:05 2012 @@ -98,6 +98,8 @@ Trunk (unreleased changes) BOOKKEEPER-424: Bookie start is failing intermittently when zkclient connection delays (rakeshr via ivank) + BOOKKEEPER-416: LedgerChecker returns underreplicated fragments for an closed ledger with no entries (ivank) + hedwig-protocol: BOOKKEEPER-394: CompositeException message is not useful (Stu Hood via sijie) Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java?rev=1402172&r1=1402171&r2=1402172&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java Thu Oct 25 14:50:05 2012 @@ -192,7 +192,8 @@ public class LedgerChecker { /* Checking the last segment of the ledger can be complicated in some cases. * In the case that the ledger is closed, we can just check the fragments of - * the segment as normal. + * the segment as normal, except in the case that no entry was ever written, + * to the ledger, in which case we check no fragments. * In the case that the ledger is open, but enough entries have been written, * for lastAddConfirmed to be set above the start entry of the segment, we * can also check as normal. @@ -203,7 +204,9 @@ public class LedgerChecker { * NoSuchEntry we can assume it was never written. If they respond with anything * else, we must assume the entry has been written, so we run the check. */ - if (curEntryId != null) { + if (curEntryId != null + && !(lh.getLastAddConfirmed() == LedgerHandle.INVALID_ENTRY_ID + && lh.getLedgerMetadata().isClosed())) { long lastEntry = lh.getLastAddConfirmed(); if (lastEntry < curEntryId) { Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java?rev=1402172&r1=1402171&r2=1402172&view=diff ============================================================================== --- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java (original) +++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java Thu Oct 25 14:50:05 2012 @@ -353,6 +353,97 @@ public class TestLedgerChecker extends B + result, 3, result.size()); } + /** + * Tests that LedgerChecker does not return any fragments + * from a closed ledger with 0 entries. + */ + @Test(timeout = 3000) + public void testClosedEmptyLedger() throws Exception { + LedgerHandle lh = bkc.createLedger(3, 3, BookKeeper.DigestType.CRC32, + TEST_LEDGER_PASSWORD); + ArrayList<InetSocketAddress> firstEnsemble = lh.getLedgerMetadata() + .getEnsembles().get(0L); + lh.close(); + + InetSocketAddress lastBookieFromEnsemble = firstEnsemble.get(0); + LOG.info("Killing " + lastBookieFromEnsemble + " from ensemble=" + + firstEnsemble); + killBookie(lastBookieFromEnsemble); + + //Open ledger separately for Ledger checker. + LedgerHandle lh1 =bkc.openLedgerNoRecovery(lh.getId(), BookKeeper.DigestType.CRC32, + TEST_LEDGER_PASSWORD); + + Set<LedgerFragment> result = getUnderReplicatedFragments(lh1); + assertNotNull("Result shouldn't be null", result); + assertEquals("There should be 0 fragment. But returned fragments are " + + result, 0, result.size()); + } + + /** + * Tests that LedgerChecker does not return any fragments + * from a closed ledger with 0 entries. + */ + @Test(timeout = 3000) + public void testClosedSingleEntryLedger() throws Exception { + LedgerHandle lh = bkc.createLedger(3, 2, BookKeeper.DigestType.CRC32, + TEST_LEDGER_PASSWORD); + ArrayList<InetSocketAddress> firstEnsemble = lh.getLedgerMetadata() + .getEnsembles().get(0L); + lh.addEntry(TEST_LEDGER_ENTRY_DATA); + lh.close(); + + // kill bookie 2 + InetSocketAddress lastBookieFromEnsemble = firstEnsemble.get(2); + LOG.info("Killing " + lastBookieFromEnsemble + " from ensemble=" + + firstEnsemble); + killBookie(lastBookieFromEnsemble); + + //Open ledger separately for Ledger checker. + LedgerHandle lh1 =bkc.openLedgerNoRecovery(lh.getId(), BookKeeper.DigestType.CRC32, + TEST_LEDGER_PASSWORD); + + Set<LedgerFragment> result = getUnderReplicatedFragments(lh1); + assertNotNull("Result shouldn't be null", result); + assertEquals("There should be 0 fragment. But returned fragments are " + + result, 0, result.size()); + lh1.close(); + + // kill bookie 1 + lastBookieFromEnsemble = firstEnsemble.get(1); + LOG.info("Killing " + lastBookieFromEnsemble + " from ensemble=" + + firstEnsemble); + killBookie(lastBookieFromEnsemble); + startNewBookie(); + + //Open ledger separately for Ledger checker. + lh1 =bkc.openLedgerNoRecovery(lh.getId(), BookKeeper.DigestType.CRC32, + TEST_LEDGER_PASSWORD); + + result = getUnderReplicatedFragments(lh1); + assertNotNull("Result shouldn't be null", result); + assertEquals("There should be 1 fragment. But returned fragments are " + + result, 1, result.size()); + lh1.close(); + + // kill bookie 0 + lastBookieFromEnsemble = firstEnsemble.get(0); + LOG.info("Killing " + lastBookieFromEnsemble + " from ensemble=" + + firstEnsemble); + killBookie(lastBookieFromEnsemble); + startNewBookie(); + + //Open ledger separately for Ledger checker. + lh1 =bkc.openLedgerNoRecovery(lh.getId(), BookKeeper.DigestType.CRC32, + TEST_LEDGER_PASSWORD); + + result = getUnderReplicatedFragments(lh1); + assertNotNull("Result shouldn't be null", result); + assertEquals("There should be 2 fragment. But returned fragments are " + + result, 2, result.size()); + lh1.close(); + } + private Set<LedgerFragment> getUnderReplicatedFragments(LedgerHandle lh) throws InterruptedException { LedgerChecker checker = new LedgerChecker(bkc);