[
https://issues.apache.org/jira/browse/CASSANDRA-3200?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16190560#comment-16190560
]
Blake Eggleston commented on CASSANDRA-3200:
--------------------------------------------
First round of review
First, I think if there’s going to be a widely used data structure that has
more than 1-2 levels of nested containers, it’s time to make some (simple)
dedicated classes. For instance, IncomingRepairStreamTracker consumes and
operates on {{Map<InetAddress, Map<InetAddress, List<Range<Token>>>>}}. What
each part of this structure represents, and the intended effect of each
collection method call is not clear. Same sort of thing with
{{Map<Range<Token>, Set<Set<InetAddress>>>}}. Rolling these structures into
classes, as well as putting the raw container manipulation behind more
meaningfully named methods will make this patch much easier to understand. It
will also allow you to test your container manipulation logic and actual
algorithm logic separately.
Some more specific stuff:
User facing:
* symmetric/asymmetric nodetool naming option is ambiguous, not sure what a
better name would be, maybe something about reducing or optimizing streams?
* should be off by default
AsymmetricSyncRequest/SyncTasks:
* Could we just add a one-way flag to the existing requests / tasks? The new
asymmetric classes duplicate most of the symmetric tasks code (I think). In the
case of local sync task, the pullRepair flag is basically doing this already.
IncomingRepairStreamTracker
* fixing the container thing as mentioned above may fix this, but it’s
difficult to figure out how this works. A top level java doc explaining how the
duplicate streams are identified and reduced would be nice.
* The class name doesn’t seem appropriate. Not all the streams are incoming,
and it’s not tracking any continuous processes. Maybe RepairStreamReducer or
RepairStreamOptimizer?
* Should be in the repair package.
IncomingRepairStreamTrackerTest
* Should throw exception instead of printing stack trace in static block
* Fix indentation of matrices in test comments
* The content of the `differences` map, as set up in testSimpleReducing doesn’t
make sense to me, why would node C be in node A’s map, but note vice versa?
* I think it would be clearer to alias the contents of addresses 0-4 to static
variables like A, B, C, etc. Parsing out the array indices when reading through
the tests is difficult to follow.
> Repair: compare all trees together (for a given range/cf) instead of by pair
> in isolation
> -----------------------------------------------------------------------------------------
>
> Key: CASSANDRA-3200
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3200
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Sylvain Lebresne
> Assignee: Marcus Eriksson
> Priority: Minor
> Labels: repair
> Fix For: 4.x
>
>
> Currently, repair compare merkle trees by pair, in isolation of any other
> tree. What that means concretely is that if I have three node A, B and C
> (RF=3) with A and B in sync, but C having some range r inconsitent with both
> A and B (since those are consistent), we will do the following transfer of r:
> A -> C, C -> A, B -> C, C -> B.
> The fact that we do both A -> C and C -> A is fine, because we cannot know
> which one is more to date from A or C. However, the transfer B -> C is
> useless provided we do A -> C if A and B are in sync. Not doing that transfer
> will be a 25% improvement in that case. With RF=5 and only one node
> inconsistent with all the others, that almost a 40% improvement, etc...
> Given that this situation of one node not in sync while the others are is
> probably fairly common (one node died so it is behind), this could be a fair
> improvement over what is transferred. In the case where we use repair to
> rebuild completely a node, this will be a dramatic improvement, because it
> will avoid the rebuilded node to get RF times the data it should get.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
