Updated Branches: refs/heads/cassandra-2.0 87aca600f -> 55b5605b7
Stop CommitLogSegment.close() from unnecessarily calling sync() prior to cleaning the buffer. Patch by belliotsmith, reviewed by marcuse for CASSANDRA-6652 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/55b5605b Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/55b5605b Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/55b5605b Branch: refs/heads/cassandra-2.0 Commit: 55b5605b7afcb7ae9bcb9b61959b91502f769db4 Parents: 87aca60 Author: Marcus Eriksson <marc...@apache.org> Authored: Tue Feb 11 09:18:09 2014 +0100 Committer: Marcus Eriksson <marc...@apache.org> Committed: Tue Feb 11 09:21:22 2014 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cassandra/db/commitlog/CommitLog.java | 2 +- .../db/commitlog/CommitLogSegment.java | 74 ++++++++++++++------ 3 files changed, 55 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/55b5605b/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index b98dec7..93552ef 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -8,6 +8,7 @@ * Correctly handle null with IF conditions and TTL (CASSANDRA-6623) * Account for range/row tombstones in tombstone drop time histogram (CASSANDRA-6522) + * Stop CommitLogSegment.close() from calling sync() (CASSANDRA-6652) Merged from 1.2: * Fix upgradesstables NPE for non-CF-based indexes (CASSANDRA-6645) * Fix partition and range deletes not triggering flush (CASSANDRA-6655) http://git-wip-us.apache.org/repos/asf/cassandra/blob/55b5605b/src/java/org/apache/cassandra/db/commitlog/CommitLog.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLog.java b/src/java/org/apache/cassandra/db/commitlog/CommitLog.java index 0e9a3f1..e9507da 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLog.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLog.java @@ -56,7 +56,7 @@ public class CommitLog implements CommitLogMBean public static final int END_OF_SEGMENT_MARKER = 0; // this is written out at the end of a segment public static final int END_OF_SEGMENT_MARKER_SIZE = 4; // number of bytes of ^^^ - public CommitLogSegment activeSegment; + public volatile CommitLogSegment activeSegment; private final CommitLogMetrics metrics; http://git-wip-us.apache.org/repos/asf/cassandra/blob/55b5605b/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java b/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java index df3d257..25658ed 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java @@ -19,8 +19,10 @@ package org.apache.cassandra.db.commitlog; import java.io.DataOutputStream; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.RandomAccessFile; +import java.nio.ByteBuffer; import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel; import java.util.Collection; @@ -130,8 +132,8 @@ public class CommitLogSegment bufferStream = new DataOutputStream(new ChecksummedOutputStream(new ByteBufferOutputStream(buffer), checksum)); buffer.putInt(CommitLog.END_OF_SEGMENT_MARKER); buffer.position(0); - needsSync = true; + sync(); } catch (IOException e) { @@ -146,8 +148,54 @@ public class CommitLogSegment { // TODO shouldn't we close the file when we're done writing to it, which comes (potentially) much earlier than it's eligible for recyling? close(); + // it's safe to simply try (and maybe fail) to delete the log file because we should only ever close()/discard() once + // the global ReplayPosition is past the current log file position, so we will never replay it; however to be on the + // safe side we attempt to rename/zero it if delete fails if (deleteFile) - FileUtils.deleteWithConfirm(logFile); + { + try + { + FileUtils.deleteWithConfirm(logFile); + } + catch (FSWriteError e) + { + // attempt to rename the file and zero its start, if possible, before throwing the error + File file = logFile; + try + { + File newFile = new File(file.getPath() + ".discarded"); + FileUtils.renameWithConfirm(file, newFile); + file = newFile; + } + catch (Throwable t) + { + } + + try + { + RandomAccessFile raf = new RandomAccessFile(file, "rw"); + ByteBuffer write = ByteBuffer.allocate(8); + write.putInt(CommitLog.END_OF_SEGMENT_MARKER); + write.position(0); + raf.getChannel().write(write); + raf.close(); + logger.error("{} {}, as we failed to delete it.", file == logFile ? "Zeroed" : "Renamed and zeroed", file); + } + catch (Throwable t) + { + if (logFile == file) + { + logger.error("Could not rename or zero {}, which we also failed to delete. In the face of other issues this could result in unnecessary log replay.", t, file); + } + else + { + logger.error("Renamed {} to {}, as we failed to delete it, however we failed to zero its header.", t, logFile, file); + } + } + throw e; + } + + } } /** @@ -157,23 +205,7 @@ public class CommitLogSegment */ public CommitLogSegment recycle() { - // writes an end-of-segment marker at the very beginning of the file and closes it - buffer.position(0); - buffer.putInt(CommitLog.END_OF_SEGMENT_MARKER); - buffer.position(0); - - try - { - sync(); - } - catch (FSWriteError e) - { - logger.error("I/O error flushing " + this + " " + e); - throw e; - } - close(); - return new CommitLogSegment(getPath()); } @@ -243,7 +275,7 @@ public class CommitLogSegment /** * Forces a disk flush for this segment file. */ - public void sync() + public synchronized void sync() { if (needsSync) { @@ -286,11 +318,12 @@ public class CommitLogSegment /** * Close the segment file. */ - public void close() + public synchronized void close() { if (closed) return; + needsSync = false; try { FileUtils.clean(buffer); @@ -382,7 +415,6 @@ public class CommitLogSegment return buffer.position(); } - public static class CommitLogSegmentFileComparator implements Comparator<File> { public int compare(File f, File f2)
