This is an automated email from the ASF dual-hosted git repository.
eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 9d6f5db 2178: IOException Should Be Thrown When Journal Is Corrupted
9d6f5db is described below
commit 9d6f5db2da75da2050ec753b4a3e7784f90c7c37
Author: Atri Sharma <[email protected]>
AuthorDate: Fri Mar 6 12:45:15 2020 +0530
2178: IOException Should Be Thrown When Journal Is Corrupted
Fixes #2178
Reviewers: Enrico Olivelli <[email protected]>, Sijie Guo
<[email protected]>
This closes #2257 from atris/2176
---
.../apache/bookkeeper/bookie/JournalChannel.java | 17 +++++-
.../bookkeeper/bookie/BookieJournalTest.java | 60 ++++++++++++++++++++++
2 files changed, 75 insertions(+), 2 deletions(-)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java
index f8a7230..2dbaee0 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java
@@ -23,6 +23,8 @@ package org.apache.bookkeeper.bookie;
import static com.google.common.base.Charsets.UTF_8;
+import com.google.common.annotations.VisibleForTesting;
+
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
@@ -30,6 +32,7 @@ import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.util.Arrays;
+
import org.apache.bookkeeper.util.NativeIO;
import org.apache.bookkeeper.util.ZeroBuffer;
import org.slf4j.Logger;
@@ -160,7 +163,7 @@ class JournalChannel implements Closeable {
+ " suddenly appeared, is another bookie process
running?");
}
randomAccessFile = new RandomAccessFile(fn, "rw");
- fc = randomAccessFile.getChannel();
+ fc = openFileChannel(randomAccessFile);
formatVersion = formatVersionToWrite;
int headerSize = (V4 == formatVersion) ? VERSION_HEADER_SIZE :
HEADER_SIZE;
@@ -178,7 +181,7 @@ class JournalChannel implements Closeable {
fc.write(zeros, nextPrealloc - journalAlignSize);
} else { // open an existing file
randomAccessFile = new RandomAccessFile(fn, "r");
- fc = randomAccessFile.getChannel();
+ fc = openFileChannel(randomAccessFile);
bc = null; // readonly
ByteBuffer bb = ByteBuffer.allocate(VERSION_HEADER_SIZE);
@@ -224,6 +227,7 @@ class JournalChannel implements Closeable {
}
} catch (IOException e) {
LOG.error("Bookie journal file can seek to position :", e);
+ throw e;
}
}
if (fRemoveFromPageCache) {
@@ -290,4 +294,13 @@ class JournalChannel implements Closeable {
this.lastDropPosition = newDropPos;
}
}
+
+ @VisibleForTesting
+ public static FileChannel openFileChannel(RandomAccessFile
randomAccessFile) {
+ if (randomAccessFile == null) {
+ throw new IllegalArgumentException("Input cannot be null");
+ }
+
+ return randomAccessFile.getChannel();
+ }
}
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
index 15589e3..88f2db7 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
@@ -47,12 +47,19 @@ import org.apache.bookkeeper.util.IOUtils;
import org.apache.commons.io.FileUtils;
import org.junit.After;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Test the bookie journal.
*/
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(JournalChannel.class)
public class BookieJournalTest {
private static final Logger LOG =
LoggerFactory.getLogger(BookieJournalTest.class);
@@ -794,4 +801,57 @@ public class BookieJournalTest {
// correct behaviour
}
}
+
+ /**
+ * Test for fake IOException during read of Journal.
+ */
+ @Test
+ public void testJournalScanIOException() throws Exception {
+ File journalDir = createTempDir("bookie", "journal");
+ Bookie.checkDirectoryStructure(Bookie.getCurrentDirectory(journalDir));
+
+ File ledgerDir = createTempDir("bookie", "ledger");
+ Bookie.checkDirectoryStructure(Bookie.getCurrentDirectory(ledgerDir));
+
+ writeV4Journal(Bookie.getCurrentDirectory(journalDir), 100,
"testPasswd".getBytes());
+
+ ServerConfiguration conf =
TestBKConfiguration.newServerConfiguration();
+ conf.setJournalDirName(journalDir.getPath())
+ .setLedgerDirNames(new String[] { ledgerDir.getPath() })
+ .setMetadataServiceUri(null);
+
+ Journal.JournalScanner journalScanner = new DummyJournalScan();
+ FileChannel fileChannel = PowerMockito.mock(FileChannel.class);
+
+ PowerMockito.when(fileChannel.position(Mockito.anyLong()))
+ .thenThrow(new IOException());
+
+ PowerMockito.mockStatic(JournalChannel.class);
+
PowerMockito.when(JournalChannel.openFileChannel(Mockito.any(RandomAccessFile.class))).thenReturn(fileChannel);
+
+ Bookie b = new Bookie(conf);
+
+ for (Journal journal : b.journals) {
+ List<Long> journalIds =
journal.listJournalIds(journal.getJournalDirectory(), null);
+
+ assertEquals(journalIds.size(), 1);
+
+ try {
+ journal.scanJournal(journalIds.get(0), Long.MAX_VALUE,
journalScanner);
+ fail("Should not have been able to scan the journal");
+ } catch (Exception e) {
+ // Expected
+ }
+ }
+
+ b.shutdown();
+ }
+
+ private class DummyJournalScan implements Journal.JournalScanner {
+
+ @Override
+ public void process(int journalVersion, long offset, ByteBuffer entry)
throws IOException {
+ LOG.warn("Journal Version : " + journalVersion);
+ }
+ };
}