[
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-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
