[ 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