[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
https://issues.apache.org/jira/browse/CASSANDRA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16411706#comment-16411706
]
Blake Eggleston commented on CASSANDRA-14058:
-
Thanks [~iamaleksey], I'll open a Jira next week to fix
> 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
>Priority: Major
> 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
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
https://issues.apache.org/jira/browse/CASSANDRA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16408101#comment-16408101
]
Aleksey Yeschenko commented on CASSANDRA-14058:
---
Noticed a couple regressions when merging up CASSANDRA-14330:
1. {{DataResolver}} no longer uses
{{cassandra.drop_oversized_readrepair_mutations}} prop - and
{{DROP_OVERSIZED_READ_REPAIR_MUTATIONS}} constant is now unused, and the
feature is missing.
2. {{RowIteratorMergeListener}} re-thrown {{AssertionError}} no longer includes
the responses. This should be restored, as without it debugging RR issues is an
even worse, potentially impossible, nightmare.
Nit: In {{DataResolver}}, {{repairResults}} field is now unused.
> 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
>Priority: Major
> 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
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
https://issues.apache.org/jira/browse/CASSANDRA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16364335#comment-16364335
]
Blake Eggleston commented on CASSANDRA-14058:
-
I'd rebased after CASSANDRA-7544 was committed and saw some dtest failures. It
should be committed soon, I just haven't had enough contiguous free time to get
everything buttoned up.
> 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
>Priority: Major
> 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
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
https://issues.apache.org/jira/browse/CASSANDRA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16333985#comment-16333985
]
Marcus Eriksson commented on CASSANDRA-14058:
-
+1, lgtm
> 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
>Priority: Major
> 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
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
https://issues.apache.org/jira/browse/CASSANDRA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16327956#comment-16327956
]
Blake Eggleston commented on CASSANDRA-14058:
-
bq. 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?
They’re intended to be BlockingReadRepair specific, since the DigestReadRepair
doesn’t do any merging. Given their size though (RIML in particular), I’d
rather not make them inner classes. Maybe moving them into repair.blocking
sub-package would be the way to go?
bq. 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.
I think I intended that as sort of a shim that would go away after
CASSANDRA-10726. Once the strategy is configurable, there will probably be a
factory class or something attached to the table metadata that can handle that
background repair case.
rebased on current trunk and pushed up with review fixes
[here|https://github.com/bdeggleston/cassandra/tree/14058-v2]
> 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
>Priority: Major
> 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
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
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
[jira] [Commented] (CASSANDRA-14058) Refactor read executor and response resolver, abstract read repair
[
https://issues.apache.org/jira/browse/CASSANDRA-14058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16257694#comment-16257694
]
Blake Eggleston commented on CASSANDRA-14058:
-
I have an initial approach here
https://github.com/bdeggleston/cassandra/tree/14058 that:
* adds a {{ReadRepair}} interface, and 2 implementations: noop and blocking
* Move read repair and short read implementation details out of DataResolver
* Added a {{service.reads}} package <- any thoughts on moving the other read
related classes from {{service}} in there?
* removes DigestMismatchException
> 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
