Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 222EB200C53 for ; Tue, 7 Mar 2017 02:20:45 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 2113A160B76; Tue, 7 Mar 2017 01:20:45 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 4DCB1160B81 for ; Tue, 7 Mar 2017 02:20:44 +0100 (CET) Received: (qmail 50950 invoked by uid 500); 7 Mar 2017 01:20:43 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 50911 invoked by uid 99); 7 Mar 2017 01:20:43 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 07 Mar 2017 01:20:43 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0F38EDFDEF; Tue, 7 Mar 2017 01:20:43 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: stefania@apache.org To: commits@cassandra.apache.org Date: Tue, 07 Mar 2017 01:20:44 -0000 Message-Id: <686e185844e340b7a8578a2f02392bbd@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [2/6] cassandra git commit: Prevent data loss on upgrade 2.1 - 3.0 by adding component separator to LogRecord absolute path archived-at: Tue, 07 Mar 2017 01:20:45 -0000 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 Authored: Mon Mar 6 09:45:56 2017 +0800 Committer: Stefania Alborghetti 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.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));