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);
+        }
+    };
 }

Reply via email to