zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iv...@apache.org
Subject 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
Date Mon, 12 Aug 2013 17:26:28 GMT
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]);



Mime
View raw message