Fixes #577. Allow for automatically blacklisting any TWCS or DTCS ta… #585
Conversation
…S or DTCS tables within a keyspace
| @JsonProperty | ||
| @NotNull | ||
| @DefaultValue("true") | ||
| private Boolean blacklistTwcsTables; |
There was a problem hiding this comment.
We're moving towards (thinking about?) keeping application-state in the storage, and keeping the configuration purely about Reaper's configuration (and not about the configuration of clusters that a user registers in a running Reaper).
That change hasn't materialised yet, but am noting it here for forward-reference sake.
There was a problem hiding this comment.
That’s great to hear. That will definitely make the app config cleaner. Plus, supporring multiple clusters is tricky for us since we need to update reaper config and cycle when new clusters are added (all contain their own unique jmx creds).
| @@ -185,6 +190,14 @@ public void setIncrementalRepair(boolean incrementalRepair) { | |||
| this.incrementalRepair = incrementalRepair; | |||
| } | |||
|
|
|||
| public boolean getBlacklistTwcsTables() { | |||
| return blacklistTwcsTables != null ? blacklistTwcsTables : false; | |||
There was a problem hiding this comment.
Isn't returning false here contradictory to the default being false?
There was a problem hiding this comment.
The default is set to true, but AFAIK that annotation is ignored by Dropwizard. I suggest to remove it and indeed keep the actual default to false.
There was a problem hiding this comment.
@denniskline can you fix this?
|
How will this work with Cassandra-4.0 and incremental repair? |
|
Yeah, very good question and I’m not sure either. I could check node version to make sure the correct default is applied. But that gets tricky when reaper handles multi-clusters on multi-versions. Incremental TWCS in 4.0 is ok, but what if the repair is scheduled to not be incremental? Should TWCS blacklist repair tables in pre-4.0 and continue to blacklist them in 4.x when non incremental is selected? |
|
@michaelsembwever - Did you want me to work on allowing TWC repairs on 4.0 before this merged in? I'm a bit confused if you want to see more added to this PR or if you like it the way it is. I'm happy to make changes. Thanks! |
It's not a blocker. I was mostly making the comment FTR. It's not so much specific to Cassandra 4.0 but to incremental repairs (yes, people are unfortunately still using incremental repairs pre Cassandra 4.0). Instead of merging master into branches could you rebase the branch off master. Otherwise we can merge this after
|
|
I took over your awesome work here @denniskline, and continued it in #602 |
…bles within a keyspace
Addressing the need to the automatically blacklist TWCS/DTCS tables per issue #577.
I decided to create a new Table object to handle both the Table.name and the Table.compactionStrategy in order to pull the required information in fewer calls. Doing this lead to additional modifications to consumers of JmxProxy
Please let me know if the approach looks good to you.