You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by st...@apache.org on 2017/03/07 01:20:44 UTC
[2/6] cassandra git commit: Prevent data loss on upgrade 2.1 - 3.0 by
adding component separator to LogRecord absolute path
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 <st...@datastax.com>
Authored: Mon Mar 6 09:45:56 2017 +0800
Committer: Stefania Alborghetti <st...@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));