[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair

["Marcus Eriksson (JIRA)" <ji...@apache.org>]

    [ 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