cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Edward Capriolo (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-6147) Allow Thrift opt-in to server-side timestamps
Date Sat, 08 Mar 2014 01:51:43 GMT

    [ 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)

Mime
View raw message