Author: jbellis
Date: Mon Apr 19 19:09:06 2010
New Revision: 935709

URL: http://svn.apache.org/viewvc?rev=935709&view=rev
Log:
avoid treating ttl <= 0 the same as ttl not specified
patch by jbellis; reviewed by Sylvain Lebresne for CASSANDRA-1003

Modified:
    cassandra/trunk/interface/cassandra.thrift
    cassandra/trunk/src/java/org/apache/cassandra/db/ExpiringColumn.java
    cassandra/trunk/src/java/org/apache/cassandra/db/RowMutation.java
    cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java
    cassandra/trunk/src/java/org/apache/cassandra/thrift/ThriftValidation.java
    cassandra/trunk/test/system/test_thrift_server.py

Modified: cassandra/trunk/interface/cassandra.thrift
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/interface/cassandra.thrift?rev=935709&r1=935708&r2=935709&view=diff
==============================================================================
--- cassandra/trunk/interface/cassandra.thrift (original)
+++ cassandra/trunk/interface/cassandra.thrift Mon Apr 19 19:09:06 2010
@@ -53,13 +53,10 @@ const string VERSION = "4.0.0"
 #
 
 /** Basic unit of data within a ColumnFamily.
- * @param name. A column name can act both as structure (a label) or as data 
(like value). Regardless, the name of the column
- *        is used as a key to its value.
- * @param value. Some data
- * @param timestamp. Used to record when data was sent to be written.
- * @param ttl. A delay (in seconds) after which the column will be 
automatically deleted. If this parameter is not
- *             provided or is <= 0, the column will never be deleted 
automatically (and will have no ttl when queried).
- *             Note that, if set, the column will be deleted from a node ttl 
seconds after the column reach the node.
+ * @param name, the name by which this column is set and retrieved.  Maximum 
64KB long.
+ * @param value. The data associated with the name.  Maximum 2GB long, but in 
practice you should limit it to small numbers of MB (since Thrift must read the 
full value into memory to operate on it).
+ * @param timestamp. The highest timestamp associated with the given column 
name is the one whose value the system will converge to.  No other assumptions 
are made about what the timestamp represents, but using 
microseconds-since-epoch is customary.
+ * @param ttl. An optional, positive delay (in seconds) after which the column 
will be automatically deleted. 
  */
 struct Column {
    1: required binary name,

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/ExpiringColumn.java
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/ExpiringColumn.java?rev=935709&r1=935708&r2=935709&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/ExpiringColumn.java 
(original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/ExpiringColumn.java Mon 
Apr 19 19:09:06 2010
@@ -51,8 +51,8 @@ public class ExpiringColumn extends Colu
     public ExpiringColumn(byte[] name, byte[] value, long timestamp, int 
timeToLive, int localExpirationTime)
     {
         super(name, value, timestamp);
-        assert timeToLive != 0;
-        assert localExpirationTime != 0;
+        assert timeToLive > 0;
+        assert localExpirationTime > 0;
         this.timeToLive = timeToLive;
         this.localExpirationTime = localExpirationTime;
     }

Modified: cassandra/trunk/src/java/org/apache/cassandra/db/RowMutation.java
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/db/RowMutation.java?rev=935709&r1=935708&r2=935709&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/db/RowMutation.java (original)
+++ cassandra/trunk/src/java/org/apache/cassandra/db/RowMutation.java Mon Apr 
19 19:09:06 2010
@@ -260,13 +260,13 @@ public class RowMutation
                     assert cosc.super_column != null;
                     for (org.apache.cassandra.thrift.Column column : 
cosc.super_column.columns)
                     {
-                        rm.add(new QueryPath(cfName, cosc.super_column.name, 
column.name), column.value, column.timestamp, column.getTtl());
+                        rm.add(new QueryPath(cfName, cosc.super_column.name, 
column.name), column.value, column.timestamp, column.ttl);
                     }
                 }
                 else
                 {
                     assert cosc.super_column == null;
-                    rm.add(new QueryPath(cfName, null, cosc.column.name), 
cosc.column.value, cosc.column.timestamp, cosc.column.getTtl());
+                    rm.add(new QueryPath(cfName, null, cosc.column.name), 
cosc.column.value, cosc.column.timestamp, cosc.column.ttl);
                 }
             }
         }

Modified: 
cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java?rev=935709&r1=935708&r2=935709&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java 
(original)
+++ cassandra/trunk/src/java/org/apache/cassandra/thrift/CassandraServer.java 
Mon Apr 19 19:09:06 2010
@@ -360,12 +360,12 @@ public class CassandraServer implements 
 
         ThriftValidation.validateKey(key);
         ThriftValidation.validateColumnParent(table, column_parent);
-        ThriftValidation.validateColumn(table, column_parent, column.name);
+        ThriftValidation.validateColumn(table, column_parent, column);
 
         RowMutation rm = new RowMutation(table, key);
         try
         {
-            rm.add(new QueryPath(column_parent.column_family, 
column_parent.super_column, column.name), column.value, column.timestamp, 
Math.max(column.ttl, 0));
+            rm.add(new QueryPath(column_parent.column_family, 
column_parent.super_column, column.name), column.value, column.timestamp, 
column.ttl);
         }
         catch (MarshalException e)
         {

Modified: 
cassandra/trunk/src/java/org/apache/cassandra/thrift/ThriftValidation.java
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/thrift/ThriftValidation.java?rev=935709&r1=935708&r2=935709&view=diff
==============================================================================
--- cassandra/trunk/src/java/org/apache/cassandra/thrift/ThriftValidation.java 
(original)
+++ cassandra/trunk/src/java/org/apache/cassandra/thrift/ThriftValidation.java 
Mon Apr 19 19:09:06 2010
@@ -208,6 +208,7 @@ public class ThriftValidation
     {
         if (cosc.column != null)
         {
+            validateTtl(cosc.column);
             ThriftValidation.validateColumnPath(keyspace, new 
ColumnPath(cfName).setSuper_column(null).setColumn(cosc.column.name));
         }
 
@@ -215,6 +216,7 @@ public class ThriftValidation
         {
             for (Column c : cosc.super_column.columns)
             {
+                validateTtl(c);
                 ThriftValidation.validateColumnPath(keyspace, new 
ColumnPath(cfName).setSuper_column(cosc.super_column.name).setColumn(c.name));
             }
         }
@@ -223,6 +225,16 @@ public class ThriftValidation
             throw new InvalidRequestException("ColumnOrSuperColumn must have 
one or both of Column or SuperColumn");
     }
 
+    private static void validateTtl(Column column) throws 
InvalidRequestException
+    {
+        if (column.isSetTtl() && column.ttl <= 0)
+        {
+            throw new InvalidRequestException("ttl must be positive");
+        }
+        // if it's not set, then it should be zero -- here we are just 
checking to make sure Thrift doesn't change that contract with us.
+        assert column.isSetTtl() || column.ttl == 0;
+    }
+
     public static void validateMutation(String keyspace, String cfName, 
Mutation mut)
             throws InvalidRequestException
     {
@@ -273,9 +285,10 @@ public class ThriftValidation
             validateColumns(keyspace, cfName, scName, predicate.column_names);
     }
 
-    public static void validateColumn(String keyspace, ColumnParent 
column_parent, byte[] column_name) throws InvalidRequestException
+    public static void validateColumn(String keyspace, ColumnParent 
column_parent, Column column) throws InvalidRequestException
     {
-        validateColumns(keyspace, column_parent, Arrays.asList(column_name));
+        validateTtl(column);
+        validateColumns(keyspace, column_parent, Arrays.asList(column.name));
     }
 
     public static void validatePredicate(String keyspace, ColumnParent 
column_parent, SlicePredicate predicate)

Modified: cassandra/trunk/test/system/test_thrift_server.py
URL: 
http://svn.apache.org/viewvc/cassandra/trunk/test/system/test_thrift_server.py?rev=935709&r1=935708&r2=935709&view=diff
==============================================================================
--- cassandra/trunk/test/system/test_thrift_server.py (original)
+++ cassandra/trunk/test/system/test_thrift_server.py Mon Apr 19 19:09:06 2010
@@ -552,6 +552,10 @@ class TestMutations(ThriftTester):
                           InvalidRequestException)
         # start > finish, key version
         _expect_exception(lambda: client.get_range_slice('Keyspace1', 
ColumnParent('Standard1'), SlicePredicate(column_names=['']), 'z', 'a', 1, 
ConsistencyLevel.ONE), InvalidRequestException)
+        # ttl must be positive
+        column = Column('cttl1', 'value1', 0, 0)
+        _expect_exception(lambda: client.insert('Keyspace1', 'key1', 
ColumnParent('Standard1'), column, ConsistencyLevel.ONE),
+                          InvalidRequestException)
 
     def test_batch_insert_super(self):
          cfmap = {'Super1': [ColumnOrSuperColumn(super_column=c) for c in 
_SUPER_COLUMNS],
@@ -994,23 +998,14 @@ class TestMutations(ThriftTester):
         client.insert('Keyspace1', 'key1', ColumnParent('Standard1'), column, 
ConsistencyLevel.ONE)
         assert client.get('Keyspace1', 'key1', ColumnPath('Standard1', 
column='cttl1'), ConsistencyLevel.ONE).column == column
 
-    def test_insert_negative_ttl(self):
-        """ Test insertion of a column with negative (invalid) ttl """
-        column = Column('cttl2', 'value1', 0, -10)
-        client.insert('Keyspace1', 'key1', ColumnParent('Standard1'), column, 
ConsistencyLevel.ONE)
-        time.sleep(0.1)
-        assert client.get('Keyspace1', 'key1', ColumnPath('Standard1', 
column='cttl2'), ConsistencyLevel.ONE).column == Column('cttl2', 'value1', 0)
-
     def test_simple_expiration(self):
         """ Test that column ttled do expires """
         column = Column('cttl3', 'value1', 0, 2)
         client.insert('Keyspace1', 'key1', ColumnParent('Standard1'), column, 
ConsistencyLevel.ONE)
         time.sleep(1)
         c = client.get('Keyspace1', 'key1', ColumnPath('Standard1', 
column='cttl3'), ConsistencyLevel.ONE).column
-        print c
-        print column
         assert c == column
-        #assert client.get('Keyspace1', 'key1', ColumnPath('Standard1', 
column='cttl3'), ConsistencyLevel.ONE).column == column
+        assert client.get('Keyspace1', 'key1', ColumnPath('Standard1', 
column='cttl3'), ConsistencyLevel.ONE).column == column
         time.sleep(2)
         _expect_missing(lambda: client.get('Keyspace1', 'key1', 
ColumnPath('Standard1', column='cttl3'), ConsistencyLevel.ONE))
 


Reply via email to