[jira] [Commented] (CASSANDRA-14621) Refactor CompactionStrategyManager
[
https://issues.apache.org/jira/browse/CASSANDRA-14621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589934#comment-16589934
]
Marcus Eriksson commented on CASSANDRA-14621:
-
+1
seems you didn't push the latest fix to circle, but the failing test passes for
me locally on the non-{{cci/csm-refactor}} branch
> Refactor CompactionStrategyManager
> --
>
> Key: CASSANDRA-14621
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14621
> Project: Cassandra
> Issue Type: Improvement
> Components: Compaction
>Reporter: Blake Eggleston
>Assignee: Blake Eggleston
>Priority: Minor
> Fix For: 4.x
>
>
> CompactionStrategyManager grew a decent amount of duplicated code as part of
> CASSANDRA-9143, which added pendingRepairs alongside the repaired and
> unrepaired buckets. At this point, the logic that routes sstables between the
> different buckets, and the different partition range divisions has gotten a
> little complex, and editing it is tedious and error prone. With transient
> replication requiring yet another bucket for, this seems like a good time to
> split some of the functionality of CSM into other classes, and make sstable
> routing a bit more generalized.
--
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-14621) Refactor CompactionStrategyManager
[ https://issues.apache.org/jira/browse/CASSANDRA-14621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16589064#comment-16589064 ] Blake Eggleston commented on CASSANDRA-14621: - pulled in your changes and pushed up some commits addressing review comments > Refactor CompactionStrategyManager > -- > > Key: CASSANDRA-14621 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14621 > Project: Cassandra > Issue Type: Improvement > Components: Compaction >Reporter: Blake Eggleston >Assignee: Blake Eggleston >Priority: Minor > Fix For: 4.x > > > CompactionStrategyManager grew a decent amount of duplicated code as part of > CASSANDRA-9143, which added pendingRepairs alongside the repaired and > unrepaired buckets. At this point, the logic that routes sstables between the > different buckets, and the different partition range divisions has gotten a > little complex, and editing it is tedious and error prone. With transient > replication requiring yet another bucket for, this seems like a good time to > split some of the functionality of CSM into other classes, and make sstable > routing a bit more generalized. -- 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-14621) Refactor CompactionStrategyManager
[
https://issues.apache.org/jira/browse/CASSANDRA-14621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16582254#comment-16582254
]
Marcus Eriksson commented on CASSANDRA-14621:
-
this lgtm in general, just a few comments
* some of the new code is non-trivial and should probably have a few tests, for
example {{AbstractStrategyHolder.GroupedSSTableContainer}} and
{{CompactionStrategyManager#groupSSTables}} (but there are probably more things
that should have tests)
* Comments on the non-obvious methods in the new classes
* Update CSM class comment
pushed a branch:
https://github.com/krummas/cassandra/commits/blake/csm-refactor containing
* nits
* noticed that we used to call startup for each strategy twice in
{{CSM#startup}}, which seems wrong, removed the call outside the {{readLock}}
* renamed {{compactionStrategyIndexFor(Descriptor descriptor)}} to
{{compactionStrategyIndexForDirectory(Descriptor descriptor)}} to make it a bit
clearer what it does
there also seems to be a few test errors which might not be unrelated
> Refactor CompactionStrategyManager
> --
>
> Key: CASSANDRA-14621
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14621
> Project: Cassandra
> Issue Type: Improvement
> Components: Compaction
>Reporter: Blake Eggleston
>Assignee: Blake Eggleston
>Priority: Minor
> Fix For: 4.x
>
>
> CompactionStrategyManager grew a decent amount of duplicated code as part of
> CASSANDRA-9143, which added pendingRepairs alongside the repaired and
> unrepaired buckets. At this point, the logic that routes sstables between the
> different buckets, and the different partition range divisions has gotten a
> little complex, and editing it is tedious and error prone. With transient
> replication requiring yet another bucket for, this seems like a good time to
> split some of the functionality of CSM into other classes, and make sstable
> routing a bit more generalized.
--
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-14621) Refactor CompactionStrategyManager
[ https://issues.apache.org/jira/browse/CASSANDRA-14621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16580044#comment-16580044 ] Marcus Eriksson commented on CASSANDRA-14621: - [~aweisberg] yeah I'll review this week > Refactor CompactionStrategyManager > -- > > Key: CASSANDRA-14621 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14621 > Project: Cassandra > Issue Type: Improvement > Components: Compaction >Reporter: Blake Eggleston >Assignee: Blake Eggleston >Priority: Minor > Fix For: 4.x > > > CompactionStrategyManager grew a decent amount of duplicated code as part of > CASSANDRA-9143, which added pendingRepairs alongside the repaired and > unrepaired buckets. At this point, the logic that routes sstables between the > different buckets, and the different partition range divisions has gotten a > little complex, and editing it is tedious and error prone. With transient > replication requiring yet another bucket for, this seems like a good time to > split some of the functionality of CSM into other classes, and make sstable > routing a bit more generalized. -- 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-14621) Refactor CompactionStrategyManager
[ https://issues.apache.org/jira/browse/CASSANDRA-14621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16579943#comment-16579943 ] Ariel Weisberg commented on CASSANDRA-14621: Fix version listed here is 4.x, but this is a dependency for transient replication. [~krummas] are you realistically going to be able to review this for 4.0? [~cscotta] ^ > Refactor CompactionStrategyManager > -- > > Key: CASSANDRA-14621 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14621 > Project: Cassandra > Issue Type: Improvement > Components: Compaction >Reporter: Blake Eggleston >Assignee: Blake Eggleston >Priority: Minor > Fix For: 4.x > > > CompactionStrategyManager grew a decent amount of duplicated code as part of > CASSANDRA-9143, which added pendingRepairs alongside the repaired and > unrepaired buckets. At this point, the logic that routes sstables between the > different buckets, and the different partition range divisions has gotten a > little complex, and editing it is tedious and error prone. With transient > replication requiring yet another bucket for, this seems like a good time to > split some of the functionality of CSM into other classes, and make sstable > routing a bit more generalized. -- 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
