[
https://issues.apache.org/jira/browse/CASSANDRA-6147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13924635#comment-13924635
]
Edward Capriolo edited comment on CASSANDRA-6147 at 3/8/14 1:50 AM:
--------------------------------------------------------------------
{quote}
We can't make the timestamp optional for remove(), but it would still work for
Deletion objects. I'm not a fan of partial support for features, but I believe
almost all Thrift clients use batch_mutate() + Deletions instead of remove()
(which is extremely limited). So, I suppose nearly-complete support of optional
timestamps is better than only having optional timestamps for inserts and no
support at all for deletes.
{quote}
I felt the same way originally. Then I revisited. remove() forces this issue.
It is rather explicit what it wants. The reality is almost everyone is doing
batch mutations, in hector even a single delete is a batch.
INSERT - OPTIONAL
INSERT BATCH - OPTIONAL
DELETE BATCH - OPTIONAL
REMOVE - NO
To me that is not "partial" support. It just happens that remove is explicit
and we can not get around that without re-working remove. I do not think anyone
would have the expectation that remove() has an optional time stamp. I added
some more documentation to the thrift file to be more clear.
I have pushed a new version which is ready for review. Usage is as follows:
{code}
@Test
public void autoTimestampsInInsert() throws InvalidRequestException,
UnavailableException, TimedOutException, NotFoundException
{
server.insert(ByteBufferUtil.bytes(ROW_NAME), COLUMN_PARENT,
getSimpleColumn(), ConsistencyLevel.ONE);
Column result = server.get(ByteBufferUtil.bytes(ROW_NAME),
pathToFindColumn() , ConsistencyLevel.ONE).getColumn();
Assert.assertTrue(result.getTimestamp() > 0);
}
@Test(expected=NotFoundException.class)
public void autoTimestampsInDeletion() throws InvalidRequestException,
UnavailableException, TimedOutException, NotFoundException
{
server.insert(ByteBufferUtil.bytes(ROW_NAME), COLUMN_PARENT,
getSimpleColumn(), ConsistencyLevel.ONE);
Deletion d = new Deletion();
d.setPredicate(new SlicePredicate());
d.getPredicate().setColumn_names(Arrays.asList(ByteBufferUtil.bytes(COLUMN_NAME)));
Mutation m = new Mutation();
m.setDeletion(d);
Map<String, List<Mutation>> muts = ImmutableMap.of(COLUMN_FAMILY,
Arrays.asList(m));
Map<ByteBuffer, Map<String, List<Mutation>>> mutation =
ImmutableMap.of(ByteBufferUtil.bytes(ROW_NAME), muts);
server.batch_mutate(mutation , ConsistencyLevel.ONE);
server.get(ByteBufferUtil.bytes(ROW_NAME), pathToFindColumn() ,
ConsistencyLevel.ONE).getColumn();
}
{code}
was (Author: appodictic):
{quote}
We can't make the timestamp optional for remove(), but it would still work for
Deletion objects. I'm not a fan of partial support for features, but I believe
almost all Thrift clients use batch_mutate() + Deletions instead of remove()
(which is extremely limited). So, I suppose nearly-complete support of optional
timestamps is better than only having optional timestamps for inserts and no
support at all for deletes.
{quote}
I felt the same way originally. Then I revisited. remove() forces this issue.
It is rather explicit what it wants. The reality is almost everyone is doing
batch mutations hector.
INSERT - OPTIONAL
INSERT BATCH - OPTIONAL
DELETE BATCH - OPTIONAL
REMOVE - NO
To me that is not "partial" support. It just happens that remove is explicit
and we can not get around that without re-working remove. I do not think anyone
would have the expectation that remove() has an optional time stamp. I added
some more documentation to the thrift file to be more clear.
I have pushed a new version which is ready for review. Usage is as follows:
{code}
@Test
public void autoTimestampsInInsert() throws InvalidRequestException,
UnavailableException, TimedOutException, NotFoundException
{
server.insert(ByteBufferUtil.bytes(ROW_NAME), COLUMN_PARENT,
getSimpleColumn(), ConsistencyLevel.ONE);
Column result = server.get(ByteBufferUtil.bytes(ROW_NAME),
pathToFindColumn() , ConsistencyLevel.ONE).getColumn();
Assert.assertTrue(result.getTimestamp() > 0);
}
@Test(expected=NotFoundException.class)
public void autoTimestampsInDeletion() throws InvalidRequestException,
UnavailableException, TimedOutException, NotFoundException
{
server.insert(ByteBufferUtil.bytes(ROW_NAME), COLUMN_PARENT,
getSimpleColumn(), ConsistencyLevel.ONE);
Deletion d = new Deletion();
d.setPredicate(new SlicePredicate());
d.getPredicate().setColumn_names(Arrays.asList(ByteBufferUtil.bytes(COLUMN_NAME)));
Mutation m = new Mutation();
m.setDeletion(d);
Map<String, List<Mutation>> muts = ImmutableMap.of(COLUMN_FAMILY,
Arrays.asList(m));
Map<ByteBuffer, Map<String, List<Mutation>>> mutation =
ImmutableMap.of(ByteBufferUtil.bytes(ROW_NAME), muts);
server.batch_mutate(mutation , ConsistencyLevel.ONE);
server.get(ByteBufferUtil.bytes(ROW_NAME), pathToFindColumn() ,
ConsistencyLevel.ONE).getColumn();
}
{code}
> Allow Thrift opt-in to server-side timestamps
> ---------------------------------------------
>
> Key: CASSANDRA-6147
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6147
> Project: Cassandra
> Issue Type: Sub-task
> Components: API
> Reporter: Edward Capriolo
> Assignee: Edward Capriolo
> Priority: Minor
> Fix For: 2.1 beta2
>
>
> Thrift users are still forced to generate timestamps on the client side.
> Currently the way the thrift bindings are generated users are forced to
> supply timestamps. There are two solutions I see.
> * -1 as timestamp means "generate on the server side"
> This is a breaking change, for those using -1 as a timestamp (which should
> effectively be no one.
> * Prepare yourself....
> Our thrift signatures are wrong, you can't overload methods in thrift
> thrift.get(byte [], byte[], ts)
> should REALLY be changed to
> GetRequest g = new GetRequest()
> g.setName()
> g.setValue()
> g.setTs() ///optional
> thrift. get( g )
> I know no one is going to want to make this change because thrift is
> quasi/dead but it would allow us to evolve thrift in a meaningful way. We
> could simple add these new methods under different names as well.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
