cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-6505) counters++ global shards 2.0 back port
Date Wed, 08 Jan 2014 19:42:51 GMT

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

Sylvain Lebresne commented on CASSANDRA-6505:
---------------------------------------------

A few small remarks on the patch:
* currently, merge doesn't do any allocation because copyTo uses ByteBufferUtil.arrayCopy
(and contexts are always array backed).  The reuse of writeElement() for copyTo makes it allocate
a BB (the duplicate()) for every shard. It's possible this wouldn't have noticeable impact
in practice, but given things like CASSANDRA-6405, I'd rather be on the safe side and save
the allocations by making writeElementAtOffset use ByteBufferUtil.arrayCopy instead of duplicating.
 This will even save some duplicate() calls on context creation compared to today as a bonus.
* Let's leave the rewrite of hasCounterId() (to use a binary search) out of this backported
patch. We should aim for "minimal amount of changes to support global shards" here and that
patch play with the border of that notion enough without that.
* Nit: Is there a reason to add GLOBAL_SHARD_INDEX_OFFSET rather than say -x-1 ("à la" binary
search) for global shards? Not that it doesn't work as it is, just that negating would seem
slightly more natural/simpler to me.
* Nit: the CounterContext class javadoc should specify how it distinguishes a local shard
from a global one. You currently have to read the code to figure it out.
* Nit: we don't need createGlobal for this patch (even tests don't use it), so let's not add
it to drive home the fact we don't create global shards.

Outside of that, it might be a bit safer to wait on CASSANDRA-6504 review (at least a first
pass) before committing this (I do plan on reviewing CASSANDRA-6504 asap, and it shouldn't
be too hard to rebase CASSANDRA-6504 on trunk + this patch in the meantime). Even if it's
very unlikely, it would suck to realize during the review of CASSANDRA-6504 that the global
shard mechanism needs some minor adjustement but that we have already released this, and I
doubt having this in 2.0.6 instead of 2.0.5 would make a huge difference.


> counters++ global shards 2.0 back port
> --------------------------------------
>
>                 Key: CASSANDRA-6505
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-6505
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>             Fix For: 2.0.5
>
>
> CASSANDRA-6504 introduces a new type of shard - 'global' - to 2.1. To enable live upgrade
from 2.0 to 2.1, it's necessary that 2.0 nodes are able to understand the new 'global' shards
in the counter contexts.
> 2.0 nodes will not produce 'global' shards, but must contain the merge logic.
> It isn't a trivial code change ("non-trivial code in a non-trivial part of the code"),
hence this separate JIRA issue.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Mime
View raw message