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/07/29 12:30:32 UTC
svn commit: r1507997 - in /zookeeper/bookkeeper/trunk: ./
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/
bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/
Author: ivank
Date: Mon Jul 29 10:30:32 2013
New Revision: 1507997
URL: http://svn.apache.org/r1507997
Log:
BOOKKEEPER-663: HierarchicalLedgerManager iterator is missing some ranges and the last ledger in the range (mmerli via ivank)
Modified:
zookeeper/bookkeeper/trunk/CHANGES.txt
zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1507997&r1=1507996&r2=1507997&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Mon Jul 29 10:30:32 2013
@@ -82,6 +82,8 @@ Trunk (unreleased changes)
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-601: readahead cache size isn't updated correctly (sijie via fpj)
Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java?rev=1507997&r1=1507996&r2=1507997&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java Mon Jul 29 10:30:32 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/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java?rev=1507997&r1=1507996&r2=1507997&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java Mon Jul 29 10:30:32 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/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java?rev=1507997&r1=1507996&r2=1507997&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java Mon Jul 29 10:30:32 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));
+ }
}