cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stefa...@apache.org
Subject [2/6] cassandra git commit: Prevent data loss on upgrade 2.1 - 3.0 by adding component separator to LogRecord absolute path
Date Tue, 07 Mar 2017 01:20:44 GMT
Prevent data loss on upgrade 2.1 - 3.0 by adding component separator to LogRecord absolute
path

patch by Stefania Alborghetti; reviewed by Marcus Eriksson for CASSANDRA-13294


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/1ba68a1e
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/1ba68a1e
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/1ba68a1e

Branch: refs/heads/cassandra-3.11
Commit: 1ba68a1e5d681c091e2c53e7720029f10591e7ef
Parents: 77d45ea
Author: Stefania Alborghetti <stefania.alborghetti@datastax.com>
Authored: Mon Mar 6 09:45:56 2017 +0800
Committer: Stefania Alborghetti <stefania.alborghetti@datastax.com>
Committed: Mon Mar 6 10:32:32 2017 +0800

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../cassandra/db/lifecycle/LogRecord.java       | 31 ++++++++++++++++----
 .../db/lifecycle/LogTransactionTest.java        |  4 +--
 3 files changed, 29 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/1ba68a1e/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 076b337..36f058b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.12
+ * Prevent data loss on upgrade 2.1 - 3.0 by adding component separator to LogRecord absolute
path (CASSANDRA-13294)
  * Improve testing on macOS by eliminating sigar logging (CASSANDRA-13233)
  * Cqlsh copy-from should error out when csv contains invalid data for collections (CASSANDRA-13071)
  * Update c.yaml doc for offheap memtables (CASSANDRA-13179)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1ba68a1e/src/java/org/apache/cassandra/db/lifecycle/LogRecord.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/lifecycle/LogRecord.java b/src/java/org/apache/cassandra/db/lifecycle/LogRecord.java
index c981b02..ac6d6d0 100644
--- a/src/java/org/apache/cassandra/db/lifecycle/LogRecord.java
+++ b/src/java/org/apache/cassandra/db/lifecycle/LogRecord.java
@@ -29,6 +29,7 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.zip.CRC32;
 
+import org.apache.cassandra.io.sstable.Component;
 import org.apache.cassandra.io.sstable.SSTable;
 import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.utils.FBUtilities;
@@ -123,7 +124,7 @@ final class LogRecord
 
             Type type = Type.fromPrefix(matcher.group(1));
             return new LogRecord(type,
-                                 matcher.group(2),
+                                 matcher.group(2) + Component.separator, // see comment on
CASSANDRA-13294 below
                                  Long.valueOf(matcher.group(3)),
                                  Integer.valueOf(matcher.group(4)),
                                  Long.valueOf(matcher.group(5)), line);
@@ -146,7 +147,11 @@ final class LogRecord
 
     public static LogRecord make(Type type, SSTable table)
     {
-        String absoluteTablePath = FileUtils.getCanonicalPath(table.descriptor.baseFilename());
+        // CASSANDRA-13294: add the sstable component separator because for legacy (2.1)
files
+        // there is no separator after the generation number, and this would cause files
of sstables with
+        // a higher generation number that starts with the same number, to be incorrectly
classified as files
+        // of this record sstable
+        String absoluteTablePath = FileUtils.getCanonicalPath(table.descriptor.baseFilename()
+ Component.separator);
         return make(type, getExistingFiles(absoluteTablePath), table.getAllFilePaths().size(),
absoluteTablePath);
     }
 
@@ -188,7 +193,7 @@ final class LogRecord
         assert !type.hasFile() || absolutePath != null : "Expected file path for file records";
 
         this.type = type;
-        this.absolutePath = type.hasFile() ? Optional.of(absolutePath) : Optional.<String>empty();
+        this.absolutePath = type.hasFile() ? Optional.of(absolutePath) : Optional.empty();
         this.updateTime = type == Type.REMOVE ? updateTime : 0;
         this.numFiles = type.hasFile() ? numFiles : 0;
         this.status = new Status();
@@ -287,9 +292,25 @@ final class LogRecord
                : false;
     }
 
-    String absolutePath()
+    /**
+     * Return the absolute path, if present, except for the last character (the descriptor
separator), or
+     * the empty string if the record has no path. This method is only to be used internally
for writing
+     * the record to file or computing the checksum.
+     *
+     * CASSANDRA-13294: the last character of the absolute path is the descriptor separator,
it is removed
+     * from the absolute path for backward compatibility, to make sure that on upgrade from
3.0.x to 3.0.y
+     * or to 3.y or to 4.0, the checksum of existing txn files still matches (in case of
non clean shutdown
+     * some txn files may be present). By removing the last character here, it means that
+     * it will never be written to txn files, but it is added after reading a txn file in
LogFile.make().
+     */
+    private String absolutePath()
     {
-        return absolutePath.isPresent() ? absolutePath.get() : "";
+        if (!absolutePath.isPresent())
+            return "";
+
+        String ret = absolutePath.get();
+        assert ret.charAt(ret.length() -1) == Component.separator : "Invalid absolute path,
should end with '-'";
+        return ret.substring(0, ret.length() - 1);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1ba68a1e/test/unit/org/apache/cassandra/db/lifecycle/LogTransactionTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/lifecycle/LogTransactionTest.java b/test/unit/org/apache/cassandra/db/lifecycle/LogTransactionTest.java
index 56df337..2544a0d 100644
--- a/test/unit/org/apache/cassandra/db/lifecycle/LogTransactionTest.java
+++ b/test/unit/org/apache/cassandra/db/lifecycle/LogTransactionTest.java
@@ -636,7 +636,7 @@ public class LogTransactionTest extends AbstractTransactionalTest
             Assert.assertEquals(2, logFiles.size());
 
             // insert a partial sstable record and a full commit record
-            String sstableRecord = LogRecord.make(LogRecord.Type.ADD, Collections.emptyList(),
0, "abc").raw;
+            String sstableRecord = LogRecord.make(LogRecord.Type.ADD, Collections.emptyList(),
0, "abc-").raw;
             int toChop = sstableRecord.length() / 2;
             FileUtils.append(logFiles.get(0), sstableRecord.substring(0, sstableRecord.length()
- toChop));
             FileUtils.append(logFiles.get(1), sstableRecord);
@@ -655,7 +655,7 @@ public class LogTransactionTest extends AbstractTransactionalTest
             Assert.assertEquals(2, logFiles.size());
 
             // insert a partial sstable record and a full commit record
-            String sstableRecord = LogRecord.make(LogRecord.Type.ADD, Collections.emptyList(),
0, "abc").raw;
+            String sstableRecord = LogRecord.make(LogRecord.Type.ADD, Collections.emptyList(),
0, "abc-").raw;
             int toChop = sstableRecord.length() / 2;
             FileUtils.append(logFiles.get(0), sstableRecord);
             FileUtils.append(logFiles.get(1), sstableRecord.substring(0, sstableRecord.length()
- toChop));


Mime
View raw message