cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcus Eriksson (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
Date Thu, 11 Jan 2018 14:40:01 GMT

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

Marcus Eriksson commented on CASSANDRA-14058:
---------------------------------------------

This looks good to me in general;

A few comments/questions/bike sheds/nits;
* {{Row/PartitionIteratorMergeListener}} has a dependency on {{BlockingReadRepair}} - guess
this should be {{ReadRepair}} instead? Or, if the {{R/PIML}} is intended to be {{BRR}} specific,
we should perhaps make them inner classes there?
* For the {{HintedReadRepair}} (CASSANDRA-10726) we can share most of the code from {{BlockingReadRepair}}
- we should probably break that out in an abstract class (but that should be done in 10726)
* Not a huge fan that {{ReadRepair}} has {{DigestResolver}}-specific methods - but I have
no real improvement suggestion here - either {{ReadRepair}} has {{DigestResolver}}-specific
logic or {{DigestResolver}} has read repair logic.
* The comment on top of {{ShortReadPartitionsProtection}} should probably go to {{ShortReadProtection}}
as that is the natural start point of SRP
* {{BlockingReadRepair.PartitionRepair}} could be a static class by passing in the size of
{{endpoints}} to the constructor
* The trace message on line 434 in {{AbstractReadExecutor}} should still log the partition
key (maybe move the trace message back to DigestResolver#responsesMatch)
* In {{AsyncOneResponse}} we could remove {{synchronized}} on {{response(..)}} (nice refactoring
of that class btw)
* {{get()}} in {{AsyncOneResponse}} should probably have {{@Override}}
* {{DigestResolver}} parameter in {{backgroundDigestRepair}} is unused

I wrote up a sloppy non-tested version of HintedReadRepair to get a feeling for the abstractions
[here|] - it contains the comments above as well (in a single big commit, sorry)

> Refactor read executor and response resolver, abstract read repair
> ------------------------------------------------------------------
>
>                 Key: CASSANDRA-14058
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14058
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Blake Eggleston
>            Assignee: Blake Eggleston
>             Fix For: 4.0
>
>
> CASSANDRA-10726 is stuck right now because the state of {{AbstractReadExecutor}} and
{{DataResolver}} make it difficult to cleanly implement. It also looks like some additional
read repair strategies might be added. This goal of this ticket is to clean up the structure
of some of the read path components to make CASSANDRA-10726 doable, and additional read repair
strategies possible.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message