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