[
https://issues.apache.org/jira/browse/CASSANDRA-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sylvain Lebresne updated CASSANDRA-1546:
----------------------------------------
Attachment: 0003-v3-Thrift-changes.patch
0002-v3-Counters.patch
0001-v3-Remove-IClock-from-internals.patch
Kelvin,
Thanks for your comment.
{quote}
The counter storage strategy moves the complexity away from the column class
and into the surrounding mutation and read logic. The problem this causes is
that to understand a column-specific property--it's partitioned value, we have
to read code in mutation and read commands and put together the logic in
conjunction w/ how it interacts w/ the cassandra data model, at large.
{quote}
While this is not untrue, the code you are mentioning is simpler than the
complex counter manipulation and I think, has a strong cassandra flavor. I
don't think that having the code slightly more split is a big deal (but I
could be wrong as I'm probably not objective on this). Imho, as far as code
complexity is involved, I'm still on the side of the actual patch (but that's
clearly debatable).
{quote}
The other challenge is that the existing cassandra data model is co-opted, so
the end user has to learn a modified data model that is counter-specific. I
don't believe this is the right encapsulation.
{quote}
It's true. However I would mitigate that with two points:
# The API for counters in the last patch is pretty close to the standard
one. The main difference being that there is no super columns support. The
other difference is the fact that there is a optional uuid on inserts.
But I don't expect anyone familiar with the Cassandra data model to be
thrown off balance by the counter API of this patch.
# I think that, in any case, it is a good idea to have a specific API for
counters, at the very least for insertions. First because we need it for
uuids (and I do believe on the usefulness of uuids, see below). Second,
cause the guarantees on counter inserts are not the usual ones. Hence, to
not make that very clear by having a specific client API call is imho a
mistake. More anecdotally, if you make counters fit in the standard API,
you force client to pack/unpack the counter value, which you can avoid
with a specific API.
So, while I agree that 1546 forces a specific counter thrift API while a
context-based approach would not necessarily, I ask the question: do we want a
specific counter API or not. But maybe I'm the only one to think that a
specific API could actually be a good idea :)
{quote}
A better encapsulation would be to re-use the context-based partition value
from #1072 in a custom Column sub-class. All the logic has already been
implemented, they just need to be refactored out of the clock class. I
understand that the context-based logic is relatively complex.
{quote}
Despite what's above, I'm not totally opposed to that. In the end, I just
want what's best for Cassandra. And I agree that it wouldn't be too hard to do
and also that it's tested already and that is good.
The complexity is still something I would be happy to avoid, even if well
tested, as it's something we would have to maintain. Adding new tests is a one
shot, maintaining something complex is forever (for some definition of
forever). That being said, the context manipulation code is not crazy
complicated either.
Still, there is a few thing that comes to mind if we try such a reuse the
context of #1072:
# it doesn't support decrement as is. Not only the uuid strategy needs it,
but even not considering that, I strongly feel it would be stupid to defer
the addition of decrements. It ain't hard to add to the context, but
something to keep in mind nevertheless.
# it would be ugly to do what 1546 does to avoid the cleanContext() function
of 1072 with contexts (not undoable but ugly). Agreed the AES repair needs
fixing, but for normal write/read, the 1546 way is faster than the
cleanContext option and I feel, cleaner.
All this being said, I willing to give it a try. I can try to modify the
(newly attached) patch for #1546 to use contexts (moved into a specific Column
sub-class and adding timestamps to the context for decrements). We could then
compare the two approaches. Only thing is, don't know exactly when I'll have
the time to do that (but I'll try asap).
{quote}
The uuid strategy is worth investigating. However, a limitation of its design
is that uuids are not coordinated across replicas. i.e. you cannot re-try and
update w/ a given uuid on multiple replicas. The leader for a given uuid must
be coordinated, at the client or coordinator level, to always correspond to
the same replica. e.g. if this is done at the coordinator level, on a failed
write, the leader has to be maintained and the mutation has to go through
hinted hand-off. However, a limitation of this is that if the chosen leader
goes down permanently, there may be orphaned HH mutations. However, it at
least permits retries on the same replica and may be interesting for certain
use cases.
{quote}
I agree, the uuid strategy as implemented in the actual (v2) patch is broken.
It does try to deal with the replay on another coordinator but it doesn't work.
I've change it to something that I believe deals with this problem and is
better overall anyway. The code is attached as v3 but this comment is already
long enough as is so I'm attaching a file (marker_idea.txt) to jira that tries
to explain the basic idea.
> (Yet another) approach to counting
> ----------------------------------
>
> Key: CASSANDRA-1546
> URL: https://issues.apache.org/jira/browse/CASSANDRA-1546
> Project: Cassandra
> Issue Type: New Feature
> Components: Core
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Fix For: 0.7.0
>
> Attachments: 0001-Remove-IClock-from-internals.patch,
> 0001-v2-Remove-IClock-from-internals.patch,
> 0001-v3-Remove-IClock-from-internals.patch, 0002-Counters.patch,
> 0002-v2-Counters.patch, 0002-v3-Counters.patch,
> 0003-Generated-thrift-files-changes.patch, 0003-v2-Thrift-changes.patch,
> 0003-v3-Thrift-changes.patch, marker_idea.txt
>
>
> This could be described as a mix between CASSANDRA-1072 without clocks and
> CASSANDRA-1421.
> More details in the comment below.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
