diff --git a/src/java/main/org/apache/zookeeper/server/persistence/FilePadding.java b/src/java/main/org/apache/zookeeper/server/persistence/FilePadding.java new file mode 100644 index 00000000000..c4052e95843 --- /dev/null +++ b/src/java/main/org/apache/zookeeper/server/persistence/FilePadding.java @@ -0,0 +1,105 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server.persistence; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; + +public class FilePadding { + private static final Logger LOG; + private static long preAllocSize = 65536 * 1024; + private static final ByteBuffer fill = ByteBuffer.allocateDirect(1); + + static { + LOG = LoggerFactory.getLogger(FileTxnLog.class); + + String size = System.getProperty("zookeeper.preAllocSize"); + if (size != null) { + try { + preAllocSize = Long.parseLong(size) * 1024; + } catch (NumberFormatException e) { + LOG.warn(size + " is not a valid value for preAllocSize"); + } + } + } + + private long currentSize; + + /** + * method to allow setting preallocate size + * of log file to pad the file. + * + * @param size the size to set to in bytes + */ + public static void setPreallocSize(long size) { + preAllocSize = size; + } + + public void setCurrentSize(long currentSize) { + this.currentSize = currentSize; + } + + /** + * pad the current file to increase its size to the next multiple of preAllocSize greater than the current size and position + * + * @param fileChannel the fileChannel of the file to be padded + * @throws IOException + */ + long padFile(FileChannel fileChannel) throws IOException { + long newFileSize = calculateFileSizeWithPadding(fileChannel.position(), currentSize, preAllocSize); + if (currentSize != newFileSize) { + fileChannel.write((ByteBuffer) fill.position(0), newFileSize - fill.remaining()); + currentSize = newFileSize; + } + return currentSize; + } + + /** + * Calculates a new file size with padding. We only return a new size if + * the current file position is sufficiently close (less than 4K) to end of + * file and preAllocSize is > 0. + * + * @param position the point in the file we have written to + * @param fileSize application keeps track of the current file size + * @param preAllocSize how many bytes to pad + * @return the new file size. It can be the same as fileSize if no + * padding was done. + * @throws IOException + */ + // VisibleForTesting + public static long calculateFileSizeWithPadding(long position, long fileSize, long preAllocSize) { + // If preAllocSize is positive and we are within 4KB of the known end of the file calculate a new file size + if (preAllocSize > 0 && position + 4096 >= fileSize) { + // If we have written more than we have previously preallocated we need to make sure the new + // file size is larger than what we already have + if (position > fileSize) { + fileSize = position + preAllocSize; + fileSize -= fileSize % preAllocSize; + } else { + fileSize += preAllocSize; + } + } + + return fileSize; + } +} diff --git a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java index 72ec606a3ae..fae7f022b59 100644 --- a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java +++ b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java @@ -91,9 +91,6 @@ public class FileTxnLog implements TxnLog { private static final Logger LOG; - static long preAllocSize = 65536 * 1024; - private static final ByteBuffer fill = ByteBuffer.allocateDirect(1); - public final static int TXNLOG_MAGIC = ByteBuffer.wrap("ZKLG".getBytes()).getInt(); @@ -107,14 +104,6 @@ public class FileTxnLog implements TxnLog { static { LOG = LoggerFactory.getLogger(FileTxnLog.class); - String size = System.getProperty("zookeeper.preAllocSize"); - if (size != null) { - try { - preAllocSize = Long.parseLong(size) * 1024; - } catch (NumberFormatException e) { - LOG.warn(size + " is not a valid value for preAllocSize"); - } - } /** Local variable to read fsync.warningthresholdms into */ Long fsyncWarningThreshold; if ((fsyncWarningThreshold = Long.getLong("zookeeper.fsync.warningthresholdms")) == null) @@ -132,8 +121,8 @@ public class FileTxnLog implements TxnLog { long dbId; private LinkedList streamsToFlush = new LinkedList(); - long currentSize; File logFileWrite = null; + private FilePadding filePadding = new FilePadding(); private volatile long syncElapsedMS = -1L; @@ -146,15 +135,6 @@ public FileTxnLog(File logDir) { this.logDir = logDir; } - /** - * method to allow setting preallocate size - * of log file to pad the file. - * @param size the size to set to in bytes - */ - public static void setPreallocSize(long size) { - preAllocSize = size; - } - /** * creates a checksum algorithm to be used * @return the checksum used for this txnlog @@ -163,7 +143,6 @@ protected Checksum makeChecksumAlgorithm(){ return new Adler32(); } - /** * rollover the current log file to a new one. * @throws IOException @@ -221,10 +200,10 @@ public synchronized boolean append(TxnHeader hdr, Record txn) fhdr.serialize(oa, "fileheader"); // Make sure that the magic number is written before padding. logStream.flush(); - currentSize = fos.getChannel().position(); + filePadding.setCurrentSize(fos.getChannel().position()); streamsToFlush.add(fos); } - currentSize = padFile(fos.getChannel()); + filePadding.padFile(fos.getChannel()); byte[] buf = Util.marshallTxnEntry(hdr, txn); if (buf == null || buf.length == 0) { throw new IOException("Faulty serialization for header " + @@ -238,49 +217,6 @@ public synchronized boolean append(TxnHeader hdr, Record txn) return true; } - /** - * pad the current file to increase its size to the next multiple of preAllocSize greater than the current size and position - * @param fileChannel the fileChannel of the file to be padded - * @throws IOException - */ - private long padFile(FileChannel fileChannel) throws IOException { - long newFileSize = calculateFileSizeWithPadding(fileChannel.position(), currentSize, preAllocSize); - if (currentSize != newFileSize) { - fileChannel.write((ByteBuffer) fill.position(0), newFileSize - fill.remaining()); - currentSize = newFileSize; - } - return currentSize; - } - - /** - * Calculates a new file size with padding. We only return a new size if - * the current file position is sufficiently close (less than 4K) to end of - * file and preAllocSize is > 0. - * - * @param position the point in the file we have written to - * @param fileSize application keeps track of the current file size - * @param preAllocSize how many bytes to pad - * @return the new file size. It can be the same as fileSize if no - * padding was done. - * @throws IOException - */ - // VisibleForTesting - public static long calculateFileSizeWithPadding(long position, long fileSize, long preAllocSize) { - // If preAllocSize is positive and we are within 4KB of the known end of the file calculate a new file size - if (preAllocSize > 0 && position + 4096 >= fileSize) { - // If we have written more than we have previously preallocated we need to make sure the new - // file size is larger than what we already have - if (position > fileSize){ - fileSize = position + preAllocSize; - fileSize -= fileSize % preAllocSize; - } else { - fileSize += preAllocSize; - } - } - - return fileSize; - } - /** * Find the log file that starts at, or just before, the snapshot. Return * this and all subsequent logs. Results are ordered by zxid of file, diff --git a/src/java/main/org/apache/zookeeper/server/persistence/TxnLogTool.java b/src/java/main/org/apache/zookeeper/server/persistence/TxnLogTool.java index f302c13bf37..d653cf469ae 100644 --- a/src/java/main/org/apache/zookeeper/server/persistence/TxnLogTool.java +++ b/src/java/main/org/apache/zookeeper/server/persistence/TxnLogTool.java @@ -41,6 +41,7 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; +import java.io.PrintStream; import java.text.DateFormat; import java.util.Date; import java.util.zip.Adler32; @@ -78,8 +79,6 @@ Options getOptions() { } } - private static final Logger LOG = LoggerFactory.getLogger(TxnLogTool.class); - private File txnLogFile; private boolean recoveryMode = false; private boolean verbose = false; @@ -114,7 +113,8 @@ public void run(String[] args) throws Exception { printStat(); } - public void init(boolean recoveryMode, boolean verbose, String txnLogFileName) throws FileNotFoundException, TxnLogToolException { + public void init(boolean recoveryMode, boolean verbose, String txnLogFileName) + throws FileNotFoundException, TxnLogToolException { this.recoveryMode = recoveryMode; this.verbose = verbose; txnLogFile = new File(txnLogFileName); @@ -141,12 +141,11 @@ public void dump() throws Exception { throw new TxnLogToolException(1, "TxnLogTool is not yet initialized"); } crcFixed = 0; + FileHeader fhdr = new FileHeader(); fhdr.deserialize(logStream, "fileheader"); - if (fhdr.getMagic() != TXNLOG_MAGIC) { - System.err.println("Invalid magic number for " + txnLogFile.getName()); - System.exit(2); + throw new TxnLogToolException(2, "Invalid magic number for %s", txnLogFile.getName()); } System.out.println("ZooKeeper Transactional Log File with dbid " + fhdr.getDbid() + " txnlog format version " @@ -181,16 +180,14 @@ public void dump() throws Exception { printTxn(bytes, "CRC FIXED"); ++crcFixed; } else { - throw new IOException("CRC doesn't match " + crcValue + - " vs " + crc.getValue()); + printTxn(bytes, "CRC ERROR"); } } if (!recoveryMode || verbose) { printTxn(bytes); } if (logStream.readByte("EOR") != 'B') { - LOG.error("Last transaction was partial."); - throw new EOFException("Last transaction was partial."); + throw new TxnLogToolException(1, "Last transaction was partial."); } if (recoveryMode) { recoveryOa.writeLong(crcValue, "crcvalue"); diff --git a/src/java/test/org/apache/zookeeper/server/persistence/FileTxnLogTest.java b/src/java/test/org/apache/zookeeper/server/persistence/FileTxnLogTest.java index 5f54d0e388f..97cbc372af4 100644 --- a/src/java/test/org/apache/zookeeper/server/persistence/FileTxnLogTest.java +++ b/src/java/test/org/apache/zookeeper/server/persistence/FileTxnLogTest.java @@ -39,27 +39,27 @@ public class FileTxnLogTest extends ZKTestCase { @Test public void testInvalidPreallocSize() { Assert.assertEquals("file should not be padded", - 10 * KB, FileTxnLog.calculateFileSizeWithPadding(7 * KB, 10 * KB, 0)); + 10 * KB, FilePadding.calculateFileSizeWithPadding(7 * KB, 10 * KB, 0)); Assert.assertEquals("file should not be padded", - 10 * KB, FileTxnLog.calculateFileSizeWithPadding(7 * KB, 10 * KB, -1)); + 10 * KB, FilePadding.calculateFileSizeWithPadding(7 * KB, 10 * KB, -1)); } @Test public void testCalculateFileSizeWithPaddingWhenNotToCurrentSize() { Assert.assertEquals("file should not be padded", - 10 * KB, FileTxnLog.calculateFileSizeWithPadding(5 * KB, 10 * KB, 10 * KB)); + 10 * KB, FilePadding.calculateFileSizeWithPadding(5 * KB, 10 * KB, 10 * KB)); } @Test public void testCalculateFileSizeWithPaddingWhenCloseToCurrentSize() { Assert.assertEquals("file should be padded an additional 10 KB", - 20 * KB, FileTxnLog.calculateFileSizeWithPadding(7 * KB, 10 * KB, 10 * KB)); + 20 * KB, FilePadding.calculateFileSizeWithPadding(7 * KB, 10 * KB, 10 * KB)); } @Test public void testFileSizeGreaterThanPosition() { Assert.assertEquals("file should be padded to 40 KB", - 40 * KB, FileTxnLog.calculateFileSizeWithPadding(31 * KB, 10 * KB, 10 * KB)); + 40 * KB, FilePadding.calculateFileSizeWithPadding(31 * KB, 10 * KB, 10 * KB)); } @Test @@ -69,7 +69,7 @@ public void testPreAllocSizeSmallerThanTxnData() throws IOException { // Set a small preAllocSize (.5 MB) final int preAllocSize = 500 * KB; - fileTxnLog.setPreallocSize(preAllocSize); + FilePadding.setPreallocSize(preAllocSize); // Create dummy txn larger than preAllocSize // Since the file padding inserts a 0, we will fill the data with 0xff to ensure we corrupt the data if we put the 0 in the data diff --git a/src/java/test/org/apache/zookeeper/server/persistence/TxnLogToolTest.java b/src/java/test/org/apache/zookeeper/server/persistence/TxnLogToolTest.java index 8cc10abb8c1..f90cfbc9c54 100644 --- a/src/java/test/org/apache/zookeeper/server/persistence/TxnLogToolTest.java +++ b/src/java/test/org/apache/zookeeper/server/persistence/TxnLogToolTest.java @@ -24,18 +24,28 @@ import org.junit.Before; import org.junit.Test; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.PrintStream; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; + public class TxnLogToolTest { private static final File testData = new File( System.getProperty("test.data.dir", "build/test/data")); + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); private File mySnapDir; @Before public void setUp() throws IOException { + System.setOut(new PrintStream(outContent)); + System.setErr(new PrintStream(errContent)); File snapDir = new File(testData, "invalidsnap"); mySnapDir = ClientBase.createTmpDir(); FileUtils.copyDirectory(snapDir, mySnapDir); @@ -43,6 +53,9 @@ public void setUp() throws IOException { @After public void tearDown() throws IOException { + System.setOut(System.out); + System.setErr(System.err); + mySnapDir.setWritable(true); FileUtils.deleteDirectory(mySnapDir); } @@ -73,9 +86,8 @@ public void testInitMissingFile() throws FileNotFoundException, TxnLogTool.TxnLo @Test(expected = TxnLogTool.TxnLogToolException.class) public void testInitWithRecoveryFileExists() throws IOException, TxnLogTool.TxnLogToolException { // Arrange - File snapDir = new File(testData, "invalidsnap"); - File logfile = new File(new File(snapDir, "version-2"), "log.274"); - File recoveryFile = new File(new File(snapDir, "version-2"), "log.274.fixed"); + File logfile = new File(new File(mySnapDir, "version-2"), "log.274"); + File recoveryFile = new File(new File(mySnapDir, "version-2"), "log.274.fixed"); recoveryFile.createNewFile(); TxnLogTool lt = new TxnLogTool(); @@ -83,19 +95,7 @@ public void testInitWithRecoveryFileExists() throws IOException, TxnLogTool.TxnL lt.init(true, false, logfile.toString()); } - @Test(expected = TxnLogTool.TxnLogToolException.class) - public void testInitWithRecoveryFileNotWritable() throws IOException, TxnLogTool.TxnLogToolException { - // Arrange - File snapDir = new File(testData, "invalidsnap"); - snapDir.setWritable(false); - File logfile = new File(new File(snapDir, "version-2"), "log.274"); - TxnLogTool lt = new TxnLogTool(); - - // Act - lt.init(true, false, logfile.toString()); - } - - @Test(expected = IOException.class) + @Test public void testDumpWithCrcError() throws Exception { // Arrange File logfile = new File(new File(mySnapDir, "version-2"), "log.42"); @@ -104,6 +104,10 @@ public void testDumpWithCrcError() throws Exception { // Act lt.dump(); + + // Assert + String output = outContent.toString(); + assertThat(output, containsString("CRC ERROR - 3/6/18 11:06:09 AM CET session 0x8061fac5ddeb0000")); } @Test @@ -118,8 +122,6 @@ public void testRecoveryFixBrokenFile() throws Exception { // Assert // Should be able to dump the recovered logfile - File fixedLogDir = new File(testData, "fixedLog"); - logfile = new File(new File(mySnapDir, "version-2"), "log.42.fixed"); lt.init(false, false, logfile.toString()); lt.dump(); diff --git a/src/java/test/org/apache/zookeeper/test/ClientBase.java b/src/java/test/org/apache/zookeeper/test/ClientBase.java index 6e91ad3b6d5..1cdff2da1b3 100644 --- a/src/java/test/org/apache/zookeeper/test/ClientBase.java +++ b/src/java/test/org/apache/zookeeper/test/ClientBase.java @@ -56,6 +56,7 @@ import org.apache.zookeeper.server.ServerCnxnFactoryAccessor; import org.apache.zookeeper.server.ZKDatabase; import org.apache.zookeeper.server.ZooKeeperServer; +import org.apache.zookeeper.server.persistence.FilePadding; import org.apache.zookeeper.server.persistence.FileTxnLog; import org.apache.zookeeper.server.quorum.QuorumPeer; import org.apache.zookeeper.server.util.OSMXBean; @@ -485,7 +486,7 @@ public static void setupTestEnv() { // resulting in test Assert.failure (client timeout on first session). // set env and directly in order to handle static init/gc issues System.setProperty("zookeeper.preAllocSize", "100"); - FileTxnLog.setPreallocSize(100 * 1024); + FilePadding.setPreallocSize(100 * 1024); } protected void setUpAll() throws Exception {