[
https://issues.apache.org/jira/browse/CASSANDRA-1070?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871054#action_12871054
]
Sylvain Lebresne commented on CASSANDRA-1070:
---------------------------------------------
I'll a pass over the patch with you suggestions, but a few comments first.
{quote}
why does IClock not implement Comparable instead of adding ClockRelationship?
{quote}
That's because version clocks can be disjoint (but timestamp clocks don't).
I admit, without version clock in the patch, it may feel weird. This is one of
those
"this patch is a step before the next patch from 580".
And I've decided to remove the DISJOINT case from ClockRelationship enum from
this patch because having it will require the addition of the notion of
reconcilier, but this
seems beyond this patch to me.
{quote}
... but more importantly doesn't
IClock oldClock = atomic.getAndSet(newClock);
temporarily set the reference to the new value (even it is smaller than the
old), before fixing it to be the max? that would be a bug.
{quote}
It do look like a bug (but maybe Kelvin could also comment on this one). Just
wanted to point out that the two previous atomicSetMax (for interger and long)
seem to be hit by the same curse. I can fix it for this patch, but should we
open a ticket for the other two ?
{quote}
+ public static ColumnSerializer serializer(ClockType clockType)
the convention everywhere is that serializer() takes no arguments, the
serialization op should inspect the clocktype instead
{quote}
That not true, SuperColumn.serializer takes an AbstractType :)
But more seriously, I'll admit I'm not sure what's the best way to do that.
For serialization it's easy, we have the clock of the column. But for
deserialization, we would need somehow to write that on disk. I see two
options (at least two simple):
1) we use the 2 bytes used for the deletion & expiration flags. Feels a bit
like a hack though.
2) we add a new field in the serialized form of the column.
And the superColumn serializer would also need to know the clock type to
deserialize the markedDeletedAt field. Here we can't use any existing flags.
Overall, it seems to me like a waste of perfectly good cpu cycles and space
just to avoid having an argument to a function (not a lot, granted, but still).
Unless ... I'm not completely awaken yet and I miss something much easier.
> Update interface to use a wrapper for various clock types
> ---------------------------------------------------------
>
> Key: CASSANDRA-1070
> URL: https://issues.apache.org/jira/browse/CASSANDRA-1070
> Project: Cassandra
> Issue Type: Sub-task
> Components: Core
> Reporter: Johan Oskarsson
> Assignee: Sylvain Lebresne
> Fix For: 0.7
>
> Attachments:
> 0001-Introduces-wrapper-for-clock-types-generalizing-time.patch,
> 0001-v2-Introduces-wrapper-for-clock-types-generalizing-time.patch,
> 0001-v4-Introduces-wrapper-for-clock-types-generalizing-time.patch,
> 0001-v5-Introduces-wrapper-for-clock-types-generalizing-time.patch,
> 0001-v6-Introduces-wrapper-for-clock-types-generalizing-time.patch,
> 0002-Update-unit-tests.patch, 0002-v6-Update-unit-tests.patch,
> 0003-Update-System-tests.patch, 0003-v6-Update-System-tests.patch
>
>
> In the latest CASSANDRA-580 patch the timestamp used for conflict resolution
> has been replaced by a Clock object that can contain either timestamp or a
> context byte array. That change allows for other conflict resolution
> techniques to be used.
> The change can be broken out and submitted here in order to make
> CASSANDRA-580 more manageable.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
