zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iv...@apache.org
Subject svn commit: r1245369 - in /zookeeper/bookkeeper/trunk: CHANGES.txt bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
Date Fri, 17 Feb 2012 10:18:00 GMT
Author: ivank
Date: Fri Feb 17 10:18:00 2012
New Revision: 1245369

URL: http://svn.apache.org/viewvc?rev=1245369&view=rev
Log:
BOOKKEEPER-169: bookie hangs on reading header when encountering partial header index file
(sijie via ivank)

Modified:
    zookeeper/bookkeeper/trunk/CHANGES.txt
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1245369&r1=1245368&r2=1245369&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Fri Feb 17 10:18:00 2012
@@ -40,6 +40,8 @@ Trunk (unreleased changes)
 	
 	BOOKKEEPER-170: Bookie constructor starts a number of threads. (ivank via fpj)
 
+        BOOKKEEPER-169: bookie hangs on reading header when encountering partial header index
file (sijie via ivank)
+
       hedwig-server/
       
         BOOKKEEPER-140: Hub server doesn't subscribe remote region correctly when a region
is down. (Sijie Gou via ivank)

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java?rev=1245369&r1=1245368&r2=1245369&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
Fri Feb 17 10:18:00 2012
@@ -25,6 +25,7 @@ import java.io.File;
 import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.nio.ByteBuffer;
+import java.nio.BufferUnderflowException;
 import java.nio.channels.FileChannel;
 
 import org.slf4j.Logger;
@@ -82,7 +83,8 @@ class FileInfo {
             fc = new RandomAccessFile(lf, "rw").getChannel();
             size = fc.size();
 
-            ByteBuffer bb = ByteBuffer.allocate(1024);
+            // avoid hang on reading partial index
+            ByteBuffer bb = ByteBuffer.allocate((int)(Math.min(size, START_OF_DATA)));
             while(bb.hasRemaining()) {
                 fc.read(bb);
             }
@@ -95,8 +97,10 @@ class FileInfo {
                 throw new IOException("Incompatible ledger version " + version);
             }
             int length = bb.getInt();
-            if (length < 0 || length > bb.remaining()) {
+            if (length < 0) {
                 throw new IOException("Length " + length + " is invalid");
+            } else if (length > bb.remaining()) {
+                throw new BufferUnderflowException();
             }
             masterKey = new byte[length];
             bb.get(masterKey);
@@ -113,25 +117,41 @@ class FileInfo {
         if (masterKey == null && !exists) {
             throw new IOException(lf + " not found");
         }
-        ByteBuffer bb = ByteBuffer.allocate(1024);
+
         if (!exists) { 
             if (create) {
                 fc = new RandomAccessFile(lf, "rw").getChannel();
                 size = fc.size();
                 if (size == 0) {
-                    bb.putInt(signature);
-                    bb.putInt(headerVersion);
-                    bb.putInt(masterKey.length);
-                    bb.put(masterKey);
-                    bb.rewind();
-                    fc.write(bb);
+                    writeHeader();
                 }
             }
         } else {
-            readHeader();
+            try {
+                readHeader();
+            } catch (BufferUnderflowException buf) {
+                LOG.warn("Exception when reading header of {} : {}", lf, buf);
+                if (null != masterKey) {
+                    LOG.warn("Attempting to write header of {} again.", lf);
+                    writeHeader();
+                } else {
+                    throw new IOException("Error reading header " + lf);
+                }
+            }
         }
     }
 
+    private void writeHeader() throws IOException {
+        ByteBuffer bb = ByteBuffer.allocate((int)START_OF_DATA);
+        bb.putInt(signature);
+        bb.putInt(headerVersion);
+        bb.putInt(masterKey.length);
+        bb.put(masterKey);
+        bb.rewind();
+        fc.position(0);
+        fc.write(bb);
+    }
+
     synchronized public long size() throws IOException {
         checkOpen(false);
         long rc = size-START_OF_DATA;

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java?rev=1245369&r1=1245368&r2=1245369&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
Fri Feb 17 10:18:00 2012
@@ -54,6 +54,8 @@ import static org.junit.Assert.*;
 public class BookieJournalTest {
     static Logger LOG = LoggerFactory.getLogger(BookieJournalTest.class);
 
+    final Random r = new Random(System.currentTimeMillis());
+
     private void writeIndexFileForLedger(File indexDir, long ledgerId,
                                          byte[] masterKey)
             throws Exception {
@@ -65,6 +67,42 @@ public class BookieJournalTest {
         fi.close();
     }
 
+    private void writePartialIndexFileForLedger(File indexDir, long ledgerId,
+                                                byte[] masterKey, boolean truncateToMasterKey)
+            throws Exception {
+        File fn = new File(indexDir, LedgerCache.getLedgerName(ledgerId));
+        fn.getParentFile().mkdirs();
+        FileInfo fi = new FileInfo(fn, masterKey);
+        // force creation of index file
+        fi.write(new ByteBuffer[]{ ByteBuffer.allocate(0) }, 0);
+        fi.close();
+        // file info header
+        int headerLen = 8 + 4 + masterKey.length;
+        // truncate the index file
+        int leftSize;
+        if (truncateToMasterKey) {
+            leftSize = r.nextInt(headerLen);
+        } else {
+            leftSize = headerLen + r.nextInt(1024 - headerLen);
+        }
+        FileChannel fc = new RandomAccessFile(fn, "rw").getChannel();
+        fc.truncate(leftSize);
+        fc.close();
+    }
+
+    /**
+     * Generate meta entry with given master key
+     */
+    private ByteBuffer generateMetaEntry(long ledgerId, byte[] masterKey) {
+        ByteBuffer bb = ByteBuffer.allocate(8 + 8 + 4 + masterKey.length);
+        bb.putLong(ledgerId);
+        bb.putLong(Bookie.METAENTRY_ID_LEDGER_KEY);
+        bb.putInt(masterKey.length);
+        bb.put(masterKey);
+        bb.flip();
+        return bb;
+    }
+
     private void writeJunkJournal(File journalDir) throws Exception {
         long logId = System.currentTimeMillis();
         File fn = new File(journalDir, Long.toHexString(logId) + ".txn");
@@ -128,6 +166,35 @@ public class BookieJournalTest {
         return jc;
     }
 
+    private JournalChannel writePostV3Journal(File journalDir, int numEntries, byte[] masterKey)
throws Exception {
+        long logId = System.currentTimeMillis();
+        JournalChannel jc = new JournalChannel(journalDir, logId);
+
+        BufferedChannel bc = jc.getBufferedChannel();
+
+        byte[] data = new byte[1024];
+        Arrays.fill(data, (byte)'X');
+        long lastConfirmed = -1;
+        for (int i = 0; i <= numEntries; i++) {
+            ByteBuffer packet;
+            if (i == 0) {
+                packet = generateMetaEntry(1, masterKey);
+            } else {
+                packet = ClientUtil.generatePacket(1, i, lastConfirmed, i*data.length, data).toByteBuffer();
+            }
+            lastConfirmed = i;
+            ByteBuffer lenBuff = ByteBuffer.allocate(4);
+            lenBuff.putInt(packet.remaining());
+            lenBuff.flip();
+
+            bc.write(lenBuff);
+            bc.write(packet);
+        }
+        bc.flush(true);
+
+        return jc;
+    }
+
     /**
      * test that we can open a journal written without the magic
      * word at the start. This is for versions of bookkeeper before
@@ -383,4 +450,111 @@ public class BookieJournalTest {
         }
     }
 
+    /**
+     * Test partial index (truncate master key) with pre-v3 journals
+     */
+    @Test
+    public void testPartialFileInfoPreV3Journal1() throws Exception {
+        testPartialFileInfoPreV3Journal(true);
+    }
+
+    /**
+     * Test partial index with pre-v3 journals
+     */
+    @Test
+    public void testPartialFileInfoPreV3Journal2() throws Exception {
+        testPartialFileInfoPreV3Journal(false);
+    }
+
+    /**
+     * Test partial index file with pre-v3 journals.
+     */
+    private void testPartialFileInfoPreV3Journal(boolean truncateMasterKey)
+        throws Exception {
+        File journalDir = File.createTempFile("bookie", "journal");
+        journalDir.delete();
+        journalDir.mkdir();
+
+        File ledgerDir = File.createTempFile("bookie", "ledger");
+        ledgerDir.delete();
+        ledgerDir.mkdir();
+
+        writePreV2Journal(journalDir, 100);
+        writePartialIndexFileForLedger(ledgerDir, 1, "testPasswd".getBytes(),
+                                       truncateMasterKey);
+
+        ServerConfiguration conf = new ServerConfiguration()
+            .setZkServers(null)
+            .setJournalDirName(journalDir.getPath())
+            .setLedgerDirNames(new String[] { ledgerDir.getPath() });
+
+        if (truncateMasterKey) {
+            try {
+                Bookie b = new Bookie(conf);
+                fail("Should not reach here!");
+            } catch (IOException ie) {
+            }
+        } else {
+            Bookie b = new Bookie(conf);
+
+            b.readEntry(1, 100);
+            try {
+                b.readEntry(1, 101);
+                fail("Shouldn't have found entry 101");
+            } catch (Bookie.NoEntryException e) {
+                // correct behaviour
+            }
+        }
+    }
+
+    /**
+     * Test partial index (truncate master key) with post-v3 journals
+     */
+    @Test
+    public void testPartialFileInfoPostV3Journal1() throws Exception {
+        testPartialFileInfoPostV3Journal(true);
+    }
+
+    /**
+     * Test partial index with post-v3 journals
+     */
+    @Test
+    public void testPartialFileInfoPostV3Journal2() throws Exception {
+        testPartialFileInfoPostV3Journal(false);
+    }
+
+    /**
+     * Test partial index file with post-v3 journals.
+     */
+    private void testPartialFileInfoPostV3Journal(boolean truncateMasterKey)
+        throws Exception {
+        File journalDir = File.createTempFile("bookie", "journal");
+        journalDir.delete();
+        journalDir.mkdir();
+
+        File ledgerDir = File.createTempFile("bookie", "ledger");
+        ledgerDir.delete();
+        ledgerDir.mkdir();
+
+        byte[] masterKey = "testPasswd".getBytes();
+
+        writePostV3Journal(journalDir, 100, masterKey);
+        writePartialIndexFileForLedger(ledgerDir, 1, masterKey,
+                                       truncateMasterKey);
+
+        ServerConfiguration conf = new ServerConfiguration()
+            .setZkServers(null)
+            .setJournalDirName(journalDir.getPath())
+            .setLedgerDirNames(new String[] { ledgerDir.getPath() });
+
+        Bookie b = new Bookie(conf);
+
+        b.readEntry(1, 100);
+        try {
+            b.readEntry(1, 101);
+            fail("Shouldn't have found entry 101");
+        } catch (Bookie.NoEntryException e) {
+            // correct behaviour
+        }
+    }
 }



Mime
View raw message