diff --git a/interface/cassandra.thrift b/interface/cassandra.thrift index 9faaf4de70..ad59b2f05d 100644 --- a/interface/cassandra.thrift +++ b/interface/cassandra.thrift @@ -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, diff --git a/src/java/org/apache/cassandra/db/ExpiringColumn.java b/src/java/org/apache/cassandra/db/ExpiringColumn.java index 26979db69d..19b2f22d30 100644 --- a/src/java/org/apache/cassandra/db/ExpiringColumn.java +++ b/src/java/org/apache/cassandra/db/ExpiringColumn.java @@ -51,8 +51,8 @@ public class ExpiringColumn extends Column 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; } diff --git a/src/java/org/apache/cassandra/db/RowMutation.java b/src/java/org/apache/cassandra/db/RowMutation.java index c0b35e7efb..fe08a24a17 100644 --- a/src/java/org/apache/cassandra/db/RowMutation.java +++ b/src/java/org/apache/cassandra/db/RowMutation.java @@ -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); } } } diff --git a/src/java/org/apache/cassandra/thrift/CassandraServer.java b/src/java/org/apache/cassandra/thrift/CassandraServer.java index c4fba39c45..f405f8b843 100644 --- a/src/java/org/apache/cassandra/thrift/CassandraServer.java +++ b/src/java/org/apache/cassandra/thrift/CassandraServer.java @@ -360,12 +360,12 @@ public class CassandraServer implements Cassandra.Iface 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) { diff --git a/src/java/org/apache/cassandra/thrift/ThriftValidation.java b/src/java/org/apache/cassandra/thrift/ThriftValidation.java index d9dd62f00a..c3be77a7c5 100644 --- a/src/java/org/apache/cassandra/thrift/ThriftValidation.java +++ b/src/java/org/apache/cassandra/thrift/ThriftValidation.java @@ -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) diff --git a/test/system/test_thrift_server.py b/test/system/test_thrift_server.py index 7a1252037d..0221099fb4 100644 --- a/test/system/test_thrift_server.py +++ b/test/system/test_thrift_server.py @@ -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))