Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4CFF2187A2 for ; Wed, 6 Jan 2016 16:50:53 +0000 (UTC) Received: (qmail 71347 invoked by uid 500); 6 Jan 2016 16:50:52 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 71281 invoked by uid 500); 6 Jan 2016 16:50:52 -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 70832 invoked by uid 99); 6 Jan 2016 16:50:52 -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; Wed, 06 Jan 2016 16:50:52 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 6F10EE0AF3; Wed, 6 Jan 2016 16:50:52 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: carl@apache.org To: commits@cassandra.apache.org Date: Wed, 06 Jan 2016 16:50:53 -0000 Message-Id: <62a3c48ddaee4786ac98cbde69458bc7@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [2/6] cassandra git commit: MV timestamp should be the maximum of the values, not the minimum MV timestamp should be the maximum of the values, not the minimum patch by Carl Yeksigian; reviewed by Jake Luciani for CASSANDRA-10910 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/70c08ece Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/70c08ece Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/70c08ece Branch: refs/heads/cassandra-3.3 Commit: 70c08ece563731cd546d24541f225c888f4d02f5 Parents: f937c8b Author: Carl Yeksigian Authored: Wed Jan 6 10:41:47 2016 -0500 Committer: Carl Yeksigian Committed: Wed Jan 6 10:42:36 2016 -0500 ---------------------------------------------------------------------- .../apache/cassandra/db/view/TemporalRow.java | 65 ++++++++++++++------ .../org/apache/cassandra/cql3/ViewTest.java | 30 +++++++++ 2 files changed, 76 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/70c08ece/src/java/org/apache/cassandra/db/view/TemporalRow.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/view/TemporalRow.java b/src/java/org/apache/cassandra/db/view/TemporalRow.java index 8898857..8ee310d 100644 --- a/src/java/org/apache/cassandra/db/view/TemporalRow.java +++ b/src/java/org/apache/cassandra/db/view/TemporalRow.java @@ -22,6 +22,7 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -279,9 +280,7 @@ public class TemporalRow this.nowInSec = nowInSec; LivenessInfo liveness = row.primaryKeyLivenessInfo(); - this.viewClusteringLocalDeletionTime = minValueIfSet(viewClusteringLocalDeletionTime, row.deletion().time().localDeletionTime(), NO_DELETION_TIME); - this.viewClusteringTimestamp = minValueIfSet(viewClusteringTimestamp, liveness.timestamp(), NO_TIMESTAMP); - this.viewClusteringTtl = minValueIfSet(viewClusteringTtl, liveness.ttl(), NO_TTL); + updateLiveness(liveness.ttl(), liveness.timestamp(), row.deletion().time().localDeletionTime()); List clusteringDefs = baseCfs.metadata.clusteringColumns(); clusteringColumns = new HashMap<>(); @@ -295,6 +294,31 @@ public class TemporalRow } } + /* + * PK ts:5, ttl:1, deletion: 2 + * Col ts:4, ttl:2, deletion: 3 + * + * TTL use min, since it expires at the lowest time which we are expiring. If we have the above values, we + * would want to return 1, since the base row expires in 1 second. + * + * Timestamp uses max, as this is the time that the row has been written to the view. See CASSANDRA-10910. + * + * Local Deletion Time should use max, as this deletion will cover all previous values written. + */ + @SuppressWarnings("unchecked") + private void updateLiveness(int ttl, long timestamp, int localDeletionTime) + { + // We are returning whichever is higher from valueIfSet + // Natural order will return the max: 1.compareTo(2) < 0, so 2 is returned + // Reverse order will return the min: 1.compareTo(2) > 0, so 1 is returned + final Comparator max = Comparator.naturalOrder(); + final Comparator min = Comparator.reverseOrder(); + + this.viewClusteringTtl = valueIfSet(viewClusteringTtl, ttl, NO_TTL, min); + this.viewClusteringTimestamp = valueIfSet(viewClusteringTimestamp, timestamp, NO_TIMESTAMP, max); + this.viewClusteringLocalDeletionTime = valueIfSet(viewClusteringLocalDeletionTime, localDeletionTime, NO_DELETION_TIME, max); + } + @Override public String toString() { @@ -351,30 +375,33 @@ public class TemporalRow // If this column is part of the view's primary keys if (viewPrimaryKey.contains(identifier)) { - this.viewClusteringTtl = minValueIfSet(this.viewClusteringTtl, ttl, NO_TTL); - this.viewClusteringTimestamp = minValueIfSet(this.viewClusteringTimestamp, timestamp, NO_TIMESTAMP); - this.viewClusteringLocalDeletionTime = minValueIfSet(this.viewClusteringLocalDeletionTime, localDeletionTime, NO_DELETION_TIME); + updateLiveness(ttl, timestamp, localDeletionTime); } innerMap.get(cellPath).setVersion(new TemporalCell(value, timestamp, ttl, localDeletionTime, isNew)); } - private static int minValueIfSet(int existing, int update, int defaultValue) - { - if (existing == defaultValue) - return update; - if (update == defaultValue) - return existing; - return Math.min(existing, update); - } - - private static long minValueIfSet(long existing, long update, long defaultValue) + /** + * @return + *
    + *
  • + * If both existing and update are defaultValue, return defaultValue + *
    • + *
  • + * If only one of existing or existing are defaultValue, return the one which is not + *
    • + *
  • + * If both existing and update are not defaultValue, compare using comparator and return the higher one. + *
    • + *
+ */ + private static T valueIfSet(T existing, T update, T defaultValue, Comparator comparator) { - if (existing == defaultValue) + if (existing.equals(defaultValue)) return update; - if (update == defaultValue) + if (update.equals(defaultValue)) return existing; - return Math.min(existing, update); + return comparator.compare(existing, update) > 0 ? existing : update; } public int viewClusteringTtl() http://git-wip-us.apache.org/repos/asf/cassandra/blob/70c08ece/test/unit/org/apache/cassandra/cql3/ViewTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/ViewTest.java b/test/unit/org/apache/cassandra/cql3/ViewTest.java index 8ae21df..2e3cf5f 100644 --- a/test/unit/org/apache/cassandra/cql3/ViewTest.java +++ b/test/unit/org/apache/cassandra/cql3/ViewTest.java @@ -275,6 +275,36 @@ public class ViewTest extends CQLTester } @Test + public void testRegularColumnTimestampUpdates() throws Throwable + { + // Regression test for CASSANDRA-10910 + + createTable("CREATE TABLE %s (" + + "k int PRIMARY KEY, " + + "c int, " + + "val int)"); + + execute("USE " + keyspace()); + executeNet(protocolVersion, "USE " + keyspace()); + + createView("mv_rctstest", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE k IS NOT NULL AND c IS NOT NULL PRIMARY KEY (k,c)"); + + updateView("UPDATE %s SET c = ?, val = ? WHERE k = ?", 0, 0, 0); + updateView("UPDATE %s SET val = ? WHERE k = ?", 1, 0); + updateView("UPDATE %s SET c = ? WHERE k = ?", 1, 0); + assertRows(execute("SELECT c, k, val FROM mv_rctstest"), row(1, 0, 1)); + + updateView("TRUNCATE %s"); + + updateView("UPDATE %s USING TIMESTAMP 1 SET c = ?, val = ? WHERE k = ?", 0, 0, 0); + updateView("UPDATE %s USING TIMESTAMP 3 SET c = ? WHERE k = ?", 1, 0); + updateView("UPDATE %s USING TIMESTAMP 2 SET val = ? WHERE k = ?", 1, 0); + updateView("UPDATE %s USING TIMESTAMP 4 SET c = ? WHERE k = ?", 2, 0); + updateView("UPDATE %s USING TIMESTAMP 3 SET val = ? WHERE k = ?", 2, 0); + assertRows(execute("SELECT c, k, val FROM mv_rctstest"), row(2, 0, 2)); + } + + @Test public void testCountersTable() throws Throwable { createTable("CREATE TABLE %s (" +