[ 
https://issues.apache.org/jira/browse/CASSANDRA-8004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14196935#comment-14196935
 ] 

Aleksey Yeschenko commented on CASSANDRA-8004:
----------------------------------------------

I can't shake the feeling that we can do better with a major-er refactor, but I 
guess not so much in 2.1 - will have to wait for the 3.0 changes.

WRT new ACS methods - claiming that all of them have a default implementation, 
and thus aren't breaking compatibility, is dishonest. All our stock 
implementations do track their own sstables and thus *have to* override them, 
and so would any 3rd party compaction strategy implementation. However, given 
the importance of this change (making incremental repair usable at all), I'm 
okay with breaking it, so long as the commit makes it to 2.1.2. But please mark 
ACS#addSSTable() and ACS#removeSSTable() abstract, so that at least nobody gets 
burned silently.

Minor nits:
- this reference is redundant in DTCS addSSTable() and removeSSTable() (and 
while at it, make DTCS.options private and final, and DTCS.sstables final)
- same 'this' nit in LCS and STCS

Other than that, LGTM/+1 - and if you agree with the comments, just make the 
modifications on commit.

> Run LCS for both repaired and unrepaired data
> ---------------------------------------------
>
>                 Key: CASSANDRA-8004
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8004
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Marcus Eriksson
>            Assignee: Marcus Eriksson
>              Labels: compaction
>             Fix For: 2.1.2
>
>
> If a user has leveled compaction configured, we should run that for both the 
> unrepaired and the repaired data. I think this would make things a lot easier 
> for end users
> It would simplify migration to incremental repairs as well, if a user runs 
> incremental repair on its nice leveled unrepaired data, we wont need to drop 
> it all to L0, instead we can just start moving sstables from the unrepaired 
> leveling straight into the repaired leveling
> Idea could be to have two instances of LeveledCompactionStrategy and move 
> sstables between the instances after an incremental repair run (and let LCS 
> be totally oblivious to whether it handles repaired or unrepaired data). Same 
> should probably apply to any compaction strategy, run two instances and 
> remove all repaired/unrepaired logic from the strategy itself.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to