You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by iv...@apache.org on 2013/06/07 18:46:11 UTC

svn commit: r1490718 - in /zookeeper/bookkeeper/branches/branch-4.2: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ bookkeeper-server/src/main/java/org/apache/bookkeeper/meta...

Author: ivank
Date: Fri Jun  7 16:46:10 2013
New Revision: 1490718

URL: http://svn.apache.org/r1490718
Log:
BOOKKEEPER-596: Ledgers are gc'ed by mistake in MSLedgerManagerFactory. (sijie via ivank)

Modified:
    zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/InMemoryMetastoreCursor.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
    zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java

Modified: zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Fri Jun  7 16:46:10 2013
@@ -18,6 +18,8 @@ Release 4.2.2 - Unreleased
 
         BOOKKEEPER-595: Crash of inprocess autorecovery daemon should not take down the bookie (ivank)
 
+        BOOKKEEPER-596: Ledgers are gc'ed by mistake in MSLedgerManagerFactory. (sijie via ivank)
+
       hedwig-server:
 
         BOOKKEEPER-579: TestSubAfterCloseSub was put in a wrong package (sijie via ivank)

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java Fri Jun  7 16:46:10 2013
@@ -72,31 +72,31 @@ public class ScanAndCompareGarbageCollec
                     garbageCleaner.clean(bkLid);
                 }
             }
+            long lastEnd = -1;
+
             while(ledgerRangeIterator.hasNext()) {
                 LedgerRange lRange = ledgerRangeIterator.next();
                 Map<Long, Boolean> subBkActiveLedgers = null;
-                Long start = lRange.start();
+
+                Long start = lastEnd + 1;
                 Long end = lRange.end();
-                if (end != LedgerRange.NOLIMIT) {
-                    subBkActiveLedgers = bkActiveLedgersSnapshot.subMap(start,
-                            true, end, true);
-                } else {
-                    if (start != LedgerRange.NOLIMIT) {
-                        subBkActiveLedgers = bkActiveLedgersSnapshot.tailMap(start);
-                    } else {
-                        subBkActiveLedgers = bkActiveLedgersSnapshot;
-                    }
+                if (!ledgerRangeIterator.hasNext()) {
+                    end = Long.MAX_VALUE;
                 }
-                Set<Long> globalActiveLedgers = lRange.getLedgers();
-                LOG.debug("All active ledgers for hash node {}, Current active ledgers from Bookie for hash node {}",
-                        globalActiveLedgers, subBkActiveLedgers.keySet());
+                subBkActiveLedgers = bkActiveLedgersSnapshot.subMap(
+                        start, true, end, true);
+
+                Set<Long> ledgersInMetadata = lRange.getLedgers();
+                LOG.debug("Active in metadata {}, Active in bookie {}",
+                          ledgersInMetadata, subBkActiveLedgers.keySet());
                 for (Long bkLid : subBkActiveLedgers.keySet()) {
-                    if (!globalActiveLedgers.contains(bkLid)) {
+                    if (!ledgersInMetadata.contains(bkLid)) {
                         // remove it from current active ledger
                         subBkActiveLedgers.remove(bkLid);
                         garbageCleaner.clean(bkLid);
                     }
                 }
+                lastEnd = end;
             }
         } catch (Exception e) {
             // ignore exception, collecting garbage next time

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java Fri Jun  7 16:46:10 2013
@@ -18,7 +18,8 @@
 package org.apache.bookkeeper.meta;
 
 import java.io.IOException;
-import java.util.HashSet;
+import java.util.TreeSet;
+import java.util.SortedSet;
 import java.util.List;
 import java.util.Set;
 
@@ -275,8 +276,8 @@ abstract class AbstractZkLedgerManager i
      *          the prefix path of the ledger nodes
      * @return ledger id hash set
      */
-    protected Set<Long> ledgerListToSet(List<String> ledgerNodes, String path) {
-        Set<Long> zkActiveLedgers = new HashSet<Long>(ledgerNodes.size(), 1.0f);
+    protected SortedSet<Long> ledgerListToSet(List<String> ledgerNodes, String path) {
+        SortedSet<Long> zkActiveLedgers = new TreeSet<Long>();
         for (String ledgerNode : ledgerNodes) {
             if (isSpecialZnode(ledgerNode)) {
                 continue;

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java Fri Jun  7 16:46:10 2013
@@ -128,25 +128,38 @@ class FlatLedgerManager extends Abstract
     public LedgerRangeIterator getLedgerRanges() {
         return new LedgerRangeIterator() {
             // single iterator, can visit only one time
-            boolean hasMoreElement = true;
-            @Override
-            public boolean hasNext() {
-                return hasMoreElement;
-            }
-            @Override
-            public LedgerRange next() throws IOException {
-                if (!hasMoreElement) {
-                    throw new NoSuchElementException();
+            boolean nextCalled = false;
+            LedgerRange nextRange = null;
+
+            synchronized private void preload() throws IOException {
+                if (nextRange != null) {
+                    return;
                 }
-                hasMoreElement = false;
-                Set<Long> zkActiveLedgers;
+                Set<Long> zkActiveLedgers = null;
+
                 try {
                     zkActiveLedgers = ledgerListToSet(
                             ZkUtils.getChildrenInSingleNode(zk, ledgerRootPath), ledgerRootPath);
-                } catch (InterruptedException e) {
-                    throw new IOException("Error when get child nodes from zk", e);
+                    nextRange = new LedgerRange(zkActiveLedgers);
+                } catch (InterruptedException ie) {
+                    Thread.currentThread().interrupt();
+                    throw new IOException("Error when get child nodes from zk", ie);
+                }
+            }
+
+            @Override
+            synchronized public boolean hasNext() throws IOException {
+                preload();
+                return nextRange != null && nextRange.size() > 0 && !nextCalled;
+            }
+
+            @Override
+            synchronized public LedgerRange next() throws IOException {
+                if (!hasNext()) {
+                    throw new NoSuchElementException();
                 }
-                return new LedgerRange(zkActiveLedgers);
+                nextCalled = true;
+                return nextRange;
             }
         };
     }

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java Fri Jun  7 16:46:10 2013
@@ -25,7 +25,7 @@ import java.util.concurrent.atomic.Atomi
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
-import java.util.Set;
+import java.util.SortedSet;
 
 import org.apache.bookkeeper.client.LedgerMetadata;
 import org.apache.bookkeeper.conf.AbstractConfiguration;
@@ -381,7 +381,8 @@ class HierarchicalLedgerManager extends 
         private Iterator<String> l1NodesIter = null;
         private Iterator<String> l2NodesIter = null;
         private String curL1Nodes = "";
-        private boolean hasMoreElement = true;
+        private boolean iteratorDone = false;
+        private LedgerRange nextRange = null;
 
         /**
          * iterate next level1 znode
@@ -410,27 +411,47 @@ class HierarchicalLedgerManager extends 
             return true;
         }
 
-        @Override
-        public boolean hasNext() throws IOException {
-            try {
-                if (l1NodesIter == null) {
-                    l1NodesIter = zk.getChildren(ledgerRootPath, null).iterator();
-                    hasMoreElement = nextL1Node();
-                } else if (l2NodesIter == null || !l2NodesIter.hasNext()) {
-                    hasMoreElement = nextL1Node();
+        synchronized private void preload() throws IOException {
+            while (nextRange == null && !iteratorDone) {
+                boolean hasMoreElements = false;
+                try {
+                    if (l1NodesIter == null) {
+                        l1NodesIter = zk.getChildren(ledgerRootPath, null).iterator();
+                        hasMoreElements = nextL1Node();
+                    } else if (l2NodesIter == null || !l2NodesIter.hasNext()) {
+                        hasMoreElements = nextL1Node();
+                    }
+                } catch (KeeperException ke) {
+                    throw new IOException("Error preloading next range", ke);
+                } catch (InterruptedException ie) {
+                    Thread.currentThread().interrupt();
+                    throw new IOException("Interrupted while preloading", ie);
+                }
+                if (hasMoreElements) {
+                    nextRange = getLedgerRangeByLevel(curL1Nodes, l2NodesIter.next());
+                    if (nextRange.size() == 0) {
+                        nextRange = null;
+                    }
+                } else {
+                    iteratorDone = true;
                 }
-            } catch (Exception e) {
-                throw new IOException("Error when check more elements", e);
             }
-            return hasMoreElement;
         }
 
         @Override
-        public LedgerRange next() throws IOException {
-            if (!hasMoreElement) {
+        synchronized public boolean hasNext() throws IOException {
+            preload();
+            return nextRange != null && !iteratorDone;
+        }
+
+        @Override
+        synchronized public LedgerRange next() throws IOException {
+            if (!hasNext()) {
                 throw new NoSuchElementException();
             }
-            return getLedgerRangeByLevel(curL1Nodes, l2NodesIter.next());
+            LedgerRange r = nextRange;
+            nextRange = null;
+            return r;
         }
 
         /**
@@ -454,13 +475,13 @@ class HierarchicalLedgerManager extends 
             } catch (InterruptedException e) {
                 throw new IOException("Error when get child nodes from zk", e);
             }
-            Set<Long> zkActiveLedgers = ledgerListToSet(ledgerNodes, nodePath);
+            SortedSet<Long> zkActiveLedgers = ledgerListToSet(ledgerNodes, nodePath);
             if (LOG.isDebugEnabled()) {
                 LOG.debug("All active ledgers from ZK for hash node "
                           + level1 + "/" + level2 + " : " + zkActiveLedgers);
             }
-            return new LedgerRange(zkActiveLedgers,
-                    getStartLedgerIdByLevel(level1, level2), getEndLedgerIdByLevel(level1, level2));
+            return new LedgerRange(zkActiveLedgers.subSet(getStartLedgerIdByLevel(level1, level2),
+                                                          getEndLedgerIdByLevel(level1, level2)));
         }
     }
 }

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java Fri Jun  7 16:46:10 2013
@@ -21,6 +21,8 @@ package org.apache.bookkeeper.meta;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 import org.apache.zookeeper.AsyncCallback;
 import org.apache.bookkeeper.client.BKException;
@@ -127,34 +129,23 @@ public interface LedgerManager extends C
      * current scan.
      */
     public static class LedgerRange {
-        // ledger start and end ranges
-        private final long start;
-        private final long end;
-        public final static long NOLIMIT = -1;
-
         // returned ledgers
-        private Set<Long> ledgers;
+        private final SortedSet<Long> ledgers;
 
         public LedgerRange(Set<Long> ledgers) {
-            this(ledgers, NOLIMIT, NOLIMIT);
-        }
-
-        public LedgerRange(Set<Long> ledgers, long start) {
-            this(ledgers, start, NOLIMIT);
+            this.ledgers = new TreeSet<Long>(ledgers);
         }
 
-        public LedgerRange(Set<Long> ledgers, long start, long end) {
-            this.ledgers = ledgers;
-            this.start = start;
-            this.end = end;
+        public int size() {
+            return this.ledgers.size();
         }
 
         public Long start() {
-            return this.start;
+            return ledgers.first();
         }
 
         public Long end() {
-            return this.end;
+            return ledgers.last();
         }
 
         public Set<Long> getLedgers() {

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java Fri Jun  7 16:46:10 2013
@@ -463,6 +463,7 @@ public class MSLedgerManagerFactory exte
         class MSLedgerRangeIterator implements LedgerRangeIterator {
             final CountDownLatch openCursorLatch = new CountDownLatch(1);
             MetastoreCursor cursor = null;
+            // last ledger id in previous range
 
             MSLedgerRangeIterator() {
                 MetastoreCallback<MetastoreCursor> openCursorCb = new MetastoreCallback<MetastoreCursor>() {
@@ -480,16 +481,16 @@ public class MSLedgerManagerFactory exte
             }
 
             @Override
-            public boolean hasNext() {
+            public boolean hasNext() throws IOException {
                 try {
                     openCursorLatch.await();
                 } catch (InterruptedException ie) {
                     LOG.error("Interrupted waiting for cursor to open", ie);
                     Thread.currentThread().interrupt();
-                    return false;
+                    throw new IOException("Interrupted waiting to read range", ie);
                 }
                 if (cursor == null) {
-                    return false;
+                    throw new IOException("Failed to open ledger range cursor, check logs");
                 }
                 return cursor.hasMoreEntries();
             }
@@ -497,7 +498,7 @@ public class MSLedgerManagerFactory exte
             @Override
             public LedgerRange next() throws IOException {
                 try {
-                    Set<Long> ledgerIds = new TreeSet<Long>();
+                    SortedSet<Long> ledgerIds = new TreeSet<Long>();
                     Iterator<MetastoreTableItem> iter = cursor.readEntries(maxEntriesPerScan);
                     while (iter.hasNext()) {
                         ledgerIds.add(key2LedgerId(iter.next().getKey()));

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/InMemoryMetastoreCursor.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/InMemoryMetastoreCursor.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/InMemoryMetastoreCursor.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/InMemoryMetastoreCursor.java Fri Jun  7 16:46:10 2013
@@ -24,22 +24,25 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.NavigableMap;
 import java.util.Set;
+import java.util.SortedMap;
 import java.util.concurrent.ScheduledExecutorService;
 
 import org.apache.bookkeeper.metastore.MSException.Code;
 import org.apache.bookkeeper.versioning.Versioned;
 
+import com.google.common.collect.ImmutableSortedMap;
+
 class InMemoryMetastoreCursor implements MetastoreCursor {
 
     private final ScheduledExecutorService scheduler;
     private final Iterator<Map.Entry<String, Versioned<Value>>> iter;
     private final Set<String> fields;
 
-    public InMemoryMetastoreCursor(NavigableMap<String, Versioned<Value>> map, Set<String> fields,
+    public InMemoryMetastoreCursor(SortedMap<String, Versioned<Value>> map, Set<String> fields,
             ScheduledExecutorService scheduler) {
-        this.iter = map.entrySet().iterator();
+        // copy an map for iterator to avoid concurrent modification problem.
+        this.iter = ImmutableSortedMap.copyOfSorted(map).entrySet().iterator();
         this.fields = fields;
         this.scheduler = scheduler;
     }

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java Fri Jun  7 16:46:10 2013
@@ -27,7 +27,12 @@ import java.util.List;
 import java.util.HashSet;
 import java.util.Random;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.Queue;
+import java.util.LinkedList;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.bookkeeper.bookie.GarbageCollector;
@@ -85,6 +90,21 @@ public class GcLedgersTest extends Ledge
         }
     }
 
+    private void removeLedger(long ledgerId) throws Exception {
+        final AtomicInteger rc = new AtomicInteger(0);
+        final CountDownLatch latch = new CountDownLatch(1);
+        getLedgerManager().removeLedgerMetadata(ledgerId, Version.ANY,
+                new GenericCallback<Void>() {
+                    @Override
+                    public void operationComplete(int rc2, Void result) {
+                        rc.set(rc2);
+                        latch.countDown();
+                    }
+                   });
+        assertTrue(latch.await(10, TimeUnit.SECONDS));
+        assertEquals("Remove should have succeeded", 0, rc.get());
+    }
+
     @Test(timeout=60000)
     public void testGarbageCollectLedgers() throws Exception {
         int numLedgers = 100;
@@ -112,7 +132,7 @@ public class GcLedgersTest extends Ledge
                                 removedLedgers.notify();
                             }
                         }
-                    });
+                   });
                 removedLedgers.wait();
             }
             removedLedgers.add(ledgerId);
@@ -175,4 +195,38 @@ public class GcLedgersTest extends Ledge
             assertTrue(activeLedgers.containsKey(ledger));
         }
     }
+
+    @Test(timeout=60000)
+    public void testGcLedgersOutsideRange() throws Exception {
+        final SortedSet<Long> createdLedgers = Collections.synchronizedSortedSet(new TreeSet<Long>());
+        final Queue<Long> cleaned = new LinkedList<Long>();
+        int numLedgers = 100;
+
+        createLedgers(numLedgers, createdLedgers);
+
+        final GarbageCollector garbageCollector =
+                new ScanAndCompareGarbageCollector(getLedgerManager(), activeLedgers);
+        GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
+                @Override
+                public void clean(long ledgerId) {
+                    LOG.info("Cleaned {}", ledgerId);
+                    cleaned.add(ledgerId);
+                }
+            };
+
+        garbageCollector.gc(cleaner);
+        assertNull("Should have cleaned nothing", cleaned.poll());
+
+        long last = createdLedgers.last();
+        removeLedger(last);
+        garbageCollector.gc(cleaner);
+        assertNotNull("Should have cleaned something", cleaned.peek());
+        assertEquals("Should have cleaned last ledger" + last, (long)last, (long)cleaned.poll());
+
+        long first = createdLedgers.first();
+        removeLedger(first);
+        garbageCollector.gc(cleaner);
+        assertNotNull("Should have cleaned something", cleaned.peek());
+        assertEquals("Should have cleaned first ledger" + first, (long)first, (long)cleaned.poll());
+    }
 }

Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java?rev=1490718&r1=1490717&r2=1490718&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java Fri Jun  7 16:46:10 2013
@@ -64,7 +64,8 @@ public abstract class LedgerManagerTestC
     public static Collection<Object[]> configs() {
         return Arrays.asList(new Object[][] {
             { FlatLedgerManagerFactory.class },
-            { HierarchicalLedgerManagerFactory.class }
+            { HierarchicalLedgerManagerFactory.class },
+            { MSLedgerManagerFactory.class }
         });
     }