You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ma...@apache.org on 2014/02/11 09:23:48 UTC
git commit: Stop CommitLogSegment.close() from unnecessarily calling
sync() prior to cleaning the buffer.
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 <ma...@apache.org>
Authored: Tue Feb 11 09:18:09 2014 +0100
Committer: Marcus Eriksson <ma...@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)