[jira] Commented: (CASSANDRA-1243) Break out context code from increment counters

["Sylvain Lebresne (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-1243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12884261#action_12884261 ] 

Sylvain Lebresne commented on CASSANDRA-1243:
---------------------------------------------

First off, I do like the idea of splitting the CASSANDRA-1072 patch to make it
easier to work with. However I think that the introduction of at least
Clock.cleanContext and Clock.update() is really hard to judge based on this
patch alone (but I have some reserve below on those two anyway).

Also note that parts of the rest of this comments is probably more about
CASSANDRA-1072 than CASSANDRA-1243, but I'm too lazy to sort my comments :)

I don't know where Kelvin is with the replication parts of the increment
counters. As far as I understand things, solving the replication may amounts
to be able to distinguish between writes and repairs. If so, I believe
cleanContext will not be useful anymore, since nodes will be able to ignore
the count concerning themselves for repairs (unless I miss something). This
would be good btw since that would remove the changes to the AntiCompaction
stuffs and prevent us to mess up by forgetting some cleanContext.

In SP.mutate() and SP.mutateBlocking(), rm.updateClocks(destination) is
called. This increments by 1 the clock count for the destination. Not sure why
it's done, seems wrong to me. If I'm right, I believe the IClock.update() can
go away, at least for as far as CASSANDRA-1072 is concerned.

Also, shouldn't we get rid of the binary context in thrift/avro (until we have
actual version clock) ? About the API, not sure what to do but we should do
something. A suggestion could be to add a thrift enum 'ClockType' (that, btw,
could be reused in the CfDef structure instead of raw string).  A clock would
then be a required ClockType followed by optional other fields that would
depend on the clock type (increment counters wouldn't need any such optional
fields).

Last minor comments, there is a missing validateValueByClock in the SC case of 
ThriftValidation.validateColumnOrSuperColumn().

> Break out context code from increment counters
> ----------------------------------------------
>
>                 Key: CASSANDRA-1243
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-1243
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Johan Oskarsson
>         Attachments: CASSANDRA-1243.patch, CASSANDRA-1243.patch
>
>
> In order to make CASSANDRA-1072 easier to digest we should split out context related functionality and prepare the code base for multiple clock types. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.