cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From marc...@apache.org
Subject [1/2] git commit: Stop CommitLogSegment.close() from unnecessarily calling sync() prior to cleaning the buffer.
Date Tue, 11 Feb 2014 08:24:01 GMT
Updated Branches:
  refs/heads/trunk f05870ee8 -> 7604935ef


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/trunk
Commit: 55b5605b7afcb7ae9bcb9b61959b91502f769db4
Parents: 87aca60
Author: Marcus Eriksson <marcuse@apache.org>
Authored: Tue Feb 11 09:18:09 2014 +0100
Committer: Marcus Eriksson <marcuse@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)


Mime
View raw message