Skip to content

Commit

Permalink
ZOOKEEPER-2994. Refactor FileTxnLog's padding logic to separate class…
Browse files Browse the repository at this point in the history
… for reusability
  • Loading branch information
anmolnar committed Mar 26, 2018
1 parent 0d089cc commit 6a1ad0e
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 102 deletions.
105 changes: 105 additions & 0 deletions src/java/main/org/apache/zookeeper/server/persistence/FilePadding.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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)
Expand All @@ -132,8 +121,8 @@ public class FileTxnLog implements TxnLog {
long dbId;
private LinkedList<FileOutputStream> streamsToFlush =
new LinkedList<FileOutputStream>();
long currentSize;
File logFileWrite = null;
private FilePadding filePadding = new FilePadding();

private volatile long syncElapsedMS = -1L;

Expand All @@ -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
Expand All @@ -163,7 +143,6 @@ protected Checksum makeChecksumAlgorithm(){
return new Adler32();
}


/**
* rollover the current log file to a new one.
* @throws IOException
Expand Down Expand Up @@ -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 " +
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 "
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit 6a1ad0e

Please sign in to comment.