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 2012/02/17 11:18:00 UTC
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
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
+ }
+ }
}