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/08/12 19:26:28 UTC
svn commit: r1513206 - in /zookeeper/bookkeeper/branches/branch-4.2:
CHANGES.txt
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
Author: ivank
Date: Mon Aug 12 17:26:27 2013
New Revision: 1513206
URL: http://svn.apache.org/r1513206
Log:
BOOKKEEPER-604: Ledger storage can log an exception if GC happens concurrently. (sijie & ivank 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/LedgerCacheImpl.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.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=1513206&r1=1513205&r2=1513206&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Mon Aug 12 17:26:27 2013
@@ -54,6 +54,8 @@ Release 4.2.2 - Unreleased
BOOKKEEPER-663: HierarchicalLedgerManager iterator is missing some ranges and the last ledger in the range (mmerli via ivank)
+ BOOKKEEPER-604: Ledger storage can log an exception if GC happens concurrently. (sijie & ivank 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/LedgerCacheImpl.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java?rev=1513206&r1=1513205&r2=1513206&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java Mon Aug 12 17:26:27 2013
@@ -419,19 +419,25 @@ public class LedgerCacheImpl implements
* @throws IOException
*/
private void flushLedger(long l) throws IOException {
+ FileInfo fi = null;
+ try {
+ fi = getFileInfo(l, null);
+ flushLedger(l, fi);
+ } catch (Bookie.NoLedgerException nle) {
+ // ledger has been deleted
+ } finally {
+ if (null != fi) {
+ fi.release();
+ }
+ }
+ }
+
+ private void flushLedger(long l, FileInfo fi) throws IOException {
LinkedList<Long> firstEntryList;
synchronized(this) {
HashMap<Long, LedgerEntryPage> pageMap = pages.get(l);
if (pageMap == null || pageMap.isEmpty()) {
- FileInfo fi = null;
- try {
- fi = getFileInfo(l, null);
- fi.flushHeader();
- } finally {
- if (null != fi) {
- fi.release();
- }
- }
+ fi.flushHeader();
return;
}
firstEntryList = new LinkedList<Long>();
@@ -453,7 +459,6 @@ public class LedgerCacheImpl implements
// Now flush all the pages of a ledger
List<LedgerEntryPage> entries = new ArrayList<LedgerEntryPage>(firstEntryList.size());
- FileInfo fi = null;
try {
for(Long firstEntry: firstEntryList) {
LedgerEntryPage lep = getLedgerEntryPage(l, firstEntry, true);
@@ -468,7 +473,6 @@ public class LedgerCacheImpl implements
}
});
ArrayList<Integer> versions = new ArrayList<Integer>(entries.size());
- fi = getFileInfo(l, null);
// flush the header if necessary
fi.flushHeader();
int start = 0;
@@ -500,9 +504,6 @@ public class LedgerCacheImpl implements
for(LedgerEntryPage lep: entries) {
lep.releasePage();
}
- if (fi != null) {
- fi.release();
- }
}
}
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java?rev=1513206&r1=1513205&r2=1513206&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java Mon Aug 12 17:26:27 2013
@@ -30,6 +30,9 @@ import org.apache.bookkeeper.conf.Server
import org.apache.bookkeeper.meta.LedgerManagerFactory;
import org.apache.bookkeeper.util.BookKeeperConstants;
import org.apache.bookkeeper.util.SnapshotMap;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.LinkedBlockingQueue;
+
import org.apache.commons.io.FileUtils;
import org.junit.After;
import org.junit.Assert;
@@ -368,6 +371,79 @@ public class LedgerCacheTest extends Tes
}
}
+ /**
+ * Race where a flush would fail because a garbage collection occurred at
+ * the wrong time.
+ * {@link https://issues.apache.org/jira/browse/BOOKKEEPER-604}
+ */
+ @Test(timeout=60000)
+ public void testFlushDeleteRace() throws Exception {
+ newLedgerCache();
+ final AtomicInteger rc = new AtomicInteger(0);
+ final LinkedBlockingQueue<Long> ledgerQ = new LinkedBlockingQueue<Long>(1);
+ final byte[] masterKey = "masterKey".getBytes();
+ Thread newLedgerThread = new Thread() {
+ public void run() {
+ try {
+ for (int i = 0; i < 1000 && rc.get() == 0; i++) {
+ ledgerCache.setMasterKey(i, masterKey);
+ ledgerQ.put((long)i);
+ }
+ } catch (Exception e) {
+ rc.set(-1);
+ LOG.error("Exception in new ledger thread", e);
+ }
+ }
+ };
+ newLedgerThread.start();
+
+ Thread flushThread = new Thread() {
+ public void run() {
+ try {
+ while (true) {
+ Long id = ledgerQ.peek();
+ if (id == null) {
+ continue;
+ }
+ LOG.info("Put entry for {}", id);
+ try {
+ ledgerCache.putEntryOffset((long)id, 1, 0);
+ } catch (Bookie.NoLedgerException nle) {
+ //ignore
+ }
+ ledgerCache.flushLedger(true);
+ }
+ } catch (Exception e) {
+ rc.set(-1);
+ LOG.error("Exception in flush thread", e);
+ }
+ }
+ };
+ flushThread.start();
+
+ Thread deleteThread = new Thread() {
+ public void run() {
+ try {
+ while (true) {
+ long id = ledgerQ.take();
+ LOG.info("Deleting {}", id);
+ ledgerCache.deleteLedger(id);
+ }
+ } catch (Exception e) {
+ rc.set(-1);
+ LOG.error("Exception in delete thread", e);
+ }
+ }
+ };
+ deleteThread.start();
+
+ newLedgerThread.join();
+ assertEquals("Should have been no errors", rc.get(), 0);
+
+ deleteThread.interrupt();
+ flushThread.interrupt();
+ }
+
private ByteBuffer generateEntry(long ledger, long entry) {
byte[] data = ("ledger-" + ledger + "-" + entry).getBytes();
ByteBuffer bb = ByteBuffer.wrap(new byte[8 + 8 + data.length]);