You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2013/08/28 12:32:55 UTC

git commit: Fix NPE in cas operations

Updated Branches:
  refs/heads/cassandra-2.0.0 cf1de3112 -> 6ad62c3f4


Fix NPE in cas operations

patch by slebresne; reviewed by jbellis for CASSANDRA-5925


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

Branch: refs/heads/cassandra-2.0.0
Commit: 6ad62c3f4a44ccbf1b462075610487e771e56f73
Parents: cf1de31
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Aug 28 12:31:59 2013 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Aug 28 12:31:59 2013 +0200

----------------------------------------------------------------------
 CHANGES.txt                                             |  1 +
 .../cql3/statements/ModificationStatement.java          | 12 +++++++++---
 src/java/org/apache/cassandra/db/SystemKeyspace.java    |  3 ++-
 3 files changed, 12 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/6ad62c3f/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index b910f14..0cb223f 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,6 +4,7 @@
  * Fix dateOf() function for pre-2.0 timestamp columns (CASSANDRA-5928)
  * Fix SSTable unintentionally loads BF when opened for batch (CASSANDRA-5938)
  * Add stream session progress to JMX (CASSANDRA-4757)
+ * Fix NPE during CAS operation (CASSANDRA-5925)
 Merged from 1.2:
  * Fix getBloomFilterDiskSpaceUsed for AlwaysPresentFilter (CASSANDRA-5900)
  * Don't announce schema version until we've loaded the changes locally

http://git-wip-us.apache.org/repos/asf/cassandra/blob/6ad62c3f/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
index e9141f9..93c4438 100644
--- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
@@ -108,6 +108,8 @@ public abstract class ModificationStatement implements CQLStatement
 
     public void validate(ClientState state) throws InvalidRequestException
     {
+        if (hasConditions() && attrs.isTimestampSet())
+            throw new InvalidRequestException("Custom timestamps are not allowed when conditions are used");
     }
 
     public void addOperation(Operation op)
@@ -370,13 +372,17 @@ public abstract class ModificationStatement implements CQLStatement
             throw new InvalidRequestException("IN on the partition key is not supported with conditional updates");
 
         ColumnNameBuilder clusteringPrefix = createClusteringPrefixBuilder(variables);
-        UpdateParameters params = new UpdateParameters(cfm, variables, getTimestamp(queryState.getTimestamp(), variables), getTimeToLive(variables), null);
 
         ByteBuffer key = keys.get(0);
         ThriftValidation.validateKey(cfm, key);
 
-        ColumnFamily updates = updateForKey(key, clusteringPrefix, params);
-        ColumnFamily expected = buildConditions(key, clusteringPrefix, params);
+        UpdateParameters updParams = new UpdateParameters(cfm, variables, queryState.getTimestamp(), getTimeToLive(variables), null);
+        ColumnFamily updates = updateForKey(key, clusteringPrefix, updParams);
+
+        // When building the conditions, we should not use the TTL. It's not useful, and if a very low ttl (1 seconds) is used, it's possible
+        // for it to expire before actually build the conditions which would break since we would then test for the presence of tombstones.
+        UpdateParameters condParams = new UpdateParameters(cfm, variables, queryState.getTimestamp(), 0, null);
+        ColumnFamily expected = buildConditions(key, clusteringPrefix, condParams);
 
         ColumnFamily result = StorageProxy.cas(keyspace(),
                                                columnFamily(),

http://git-wip-us.apache.org/repos/asf/cassandra/blob/6ad62c3f/src/java/org/apache/cassandra/db/SystemKeyspace.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/SystemKeyspace.java b/src/java/org/apache/cassandra/db/SystemKeyspace.java
index dba3421..5dd505f 100644
--- a/src/java/org/apache/cassandra/db/SystemKeyspace.java
+++ b/src/java/org/apache/cassandra/db/SystemKeyspace.java
@@ -806,10 +806,11 @@ public class SystemKeyspace
 
     public static void savePaxosProposal(Commit commit)
     {
-        processInternal(String.format("UPDATE %s USING TIMESTAMP %d AND TTL %d SET proposal = 0x%s WHERE row_key = 0x%s AND cf_id = %s",
+        processInternal(String.format("UPDATE %s USING TIMESTAMP %d AND TTL %d SET in_progress_ballot = %s, proposal = 0x%s WHERE row_key = 0x%s AND cf_id = %s",
                                       PAXOS_CF,
                                       UUIDGen.microsTimestamp(commit.ballot),
                                       paxosTtl(commit.update.metadata),
+                                      commit.ballot,
                                       ByteBufferUtil.bytesToHex(commit.update.toBytes()),
                                       ByteBufferUtil.bytesToHex(commit.key),
                                       commit.update.id()));