zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iv...@apache.org
Subject svn commit: r1508005 - in /zookeeper/bookkeeper/branches/branch-4.2: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/
Date Mon, 29 Jul 2013 12:22:48 GMT
Author: ivank
Date: Mon Jul 29 12:22:48 2013
New Revision: 1508005

URL: http://svn.apache.org/r1508005
Log:
BOOKKEEPER-663: HierarchicalLedgerManager iterator is missing some ranges and the last ledger
in the range (mmerli 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/meta/AbstractZkLedgerManager.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/test/java/org/apache/bookkeeper/meta/GcLedgersTest.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=1508005&r1=1508004&r2=1508005&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Mon Jul 29 12:22:48 2013
@@ -52,6 +52,8 @@ Release 4.2.2 - Unreleased
 
         BOOKKEEPER-642: Bookie returns incorrect exitcode, ExitCode.ZK_REG_FAIL is getting
overridden (Rakesh via ivank)
 
+        BOOKKEEPER-663: HierarchicalLedgerManager iterator is missing some ranges and the
last ledger in the range (mmerli 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/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=1508005&r1=1508004&r2=1508005&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
Mon Jul 29 12:22:48 2013
@@ -18,32 +18,30 @@
 package org.apache.bookkeeper.meta;
 
 import java.io.IOException;
-import java.util.TreeSet;
-import java.util.SortedSet;
 import java.util.List;
+import java.util.NavigableSet;
 import java.util.Set;
+import java.util.TreeSet;
 
-import org.apache.bookkeeper.conf.AbstractConfiguration;
-import org.apache.bookkeeper.client.LedgerMetadata;
 import org.apache.bookkeeper.client.BKException;
-import org.apache.bookkeeper.meta.LedgerManager;
+import org.apache.bookkeeper.client.LedgerMetadata;
+import org.apache.bookkeeper.conf.AbstractConfiguration;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.MultiCallback;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.Processor;
 import org.apache.bookkeeper.util.BookKeeperConstants;
-import org.apache.bookkeeper.versioning.Version;
 import org.apache.bookkeeper.util.ZkUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
+import org.apache.bookkeeper.versioning.Version;
 import org.apache.zookeeper.AsyncCallback;
 import org.apache.zookeeper.AsyncCallback.DataCallback;
-import org.apache.zookeeper.AsyncCallback.VoidCallback;
 import org.apache.zookeeper.AsyncCallback.StatCallback;
+import org.apache.zookeeper.AsyncCallback.VoidCallback;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Abstract ledger manager based on zookeeper, which provides common methods such as query
zk nodes.
@@ -276,8 +274,8 @@ abstract class AbstractZkLedgerManager i
      *          the prefix path of the ledger nodes
      * @return ledger id hash set
      */
-    protected SortedSet<Long> ledgerListToSet(List<String> ledgerNodes, String
path) {
-        SortedSet<Long> zkActiveLedgers = new TreeSet<Long>();
+    protected NavigableSet<Long> ledgerListToSet(List<String> ledgerNodes, String
path) {
+        NavigableSet<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/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=1508005&r1=1508004&r2=1508005&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
Mon Jul 29 12:22:48 2013
@@ -19,13 +19,14 @@ package org.apache.bookkeeper.meta;
  */
 
 import java.io.IOException;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.NavigableSet;
 import java.util.NoSuchElementException;
-import java.util.SortedSet;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.bookkeeper.client.LedgerMetadata;
 import org.apache.bookkeeper.conf.AbstractConfiguration;
@@ -40,7 +41,6 @@ import org.apache.zookeeper.KeeperExcept
 import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.ZooKeeper;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -402,6 +402,7 @@ class HierarchicalLedgerManager extends 
                     continue;
                 }
                 List<String> l2Nodes = zk.getChildren(ledgerRootPath + "/" + curL1Nodes,
null);
+                Collections.sort(l2Nodes);
                 l2NodesIter = l2Nodes.iterator();
                 if (!l2NodesIter.hasNext()) {
                     l2NodesIter = null;
@@ -420,6 +421,8 @@ class HierarchicalLedgerManager extends 
                         hasMoreElements = nextL1Node();
                     } else if (l2NodesIter == null || !l2NodesIter.hasNext()) {
                         hasMoreElements = nextL1Node();
+                    } else {
+                        hasMoreElements = true;
                     }
                 } catch (KeeperException ke) {
                     throw new IOException("Error preloading next range", ke);
@@ -475,13 +478,14 @@ class HierarchicalLedgerManager extends 
             } catch (InterruptedException e) {
                 throw new IOException("Error when get child nodes from zk", e);
             }
-            SortedSet<Long> zkActiveLedgers = ledgerListToSet(ledgerNodes, nodePath);
+            NavigableSet<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.subSet(getStartLedgerIdByLevel(level1,
level2),
-                                                          getEndLedgerIdByLevel(level1, level2)));
+
+            return new LedgerRange(zkActiveLedgers.subSet(getStartLedgerIdByLevel(level1,
level2), true,
+                                                          getEndLedgerIdByLevel(level1, level2),
true));
         }
     }
 }

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=1508005&r1=1508004&r2=1508005&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
Mon Jul 29 12:22:48 2013
@@ -23,14 +23,14 @@ package org.apache.bookkeeper.meta;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.List;
 import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Queue;
 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;
@@ -38,15 +38,16 @@ import java.util.concurrent.atomic.Atomi
 import org.apache.bookkeeper.bookie.GarbageCollector;
 import org.apache.bookkeeper.bookie.ScanAndCompareGarbageCollector;
 import org.apache.bookkeeper.client.BKException;
-import org.apache.bookkeeper.client.LedgerMetadata;
 import org.apache.bookkeeper.client.BookKeeper.DigestType;
+import org.apache.bookkeeper.client.LedgerMetadata;
+import org.apache.bookkeeper.meta.LedgerManager.LedgerRange;
+import org.apache.bookkeeper.meta.LedgerManager.LedgerRangeIterator;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
 import org.apache.bookkeeper.versioning.Version;
+import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.junit.Test;
-
 /**
  * Test garbage collection ledgers in ledger manager
  */
@@ -229,4 +230,43 @@ public class GcLedgersTest extends Ledge
         assertNotNull("Should have cleaned something", cleaned.peek());
         assertEquals("Should have cleaned first ledger" + first, (long)first, (long)cleaned.poll());
     }
+
+    @Test(timeout=60000)
+    public void testGcLedgersNotLast() throws Exception {
+        final SortedSet<Long> createdLedgers = Collections.synchronizedSortedSet(new
TreeSet<Long>());
+        final List<Long> cleaned = new ArrayList<Long>();
+
+        // Create enough ledgers to span over 4 ranges in the hierarchical ledger manager
implementation
+        final int numLedgers = 30001;
+
+        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);
+                }
+            };
+
+        SortedSet<Long> scannedLedgers = new TreeSet<Long>();
+        LedgerRangeIterator iterator = getLedgerManager().getLedgerRanges();
+        while (iterator.hasNext()) {
+            LedgerRange ledgerRange = iterator.next();
+            scannedLedgers.addAll(ledgerRange.getLedgers());
+        }
+
+        assertEquals(createdLedgers, scannedLedgers);
+
+        garbageCollector.gc(cleaner);
+        assertTrue("Should have cleaned nothing", cleaned.isEmpty());
+
+        long first = createdLedgers.first();
+        removeLedger(first);
+        garbageCollector.gc(cleaner);
+        assertEquals("Should have cleaned something", 1, cleaned.size());
+        assertEquals("Should have cleaned first ledger" + first, (long)first, (long)cleaned.get(0));
+    }
 }



Mime
View raw message