Repair all keyspaces

[tiago-loureiro]

Fixes https://github.com/zinfra/backend-issues/issues/1701

Idea behind it is to avoid having defaults that may be risky (e.g., people not reading the docs) instead of simply repairing the whole cluster (all keyspaces). I am not sure there's a good enough reason to take that as an input.

The only thing it changes from what we do in production is that metrics are reported slightly differently but I don't see that as an issue.

[jschaul]

(can we also please backport this change in hegemony?)

[lucendio]

Would it make sense to allow 3 cases so that cassandra_keyspaces can be kept? (backward compatibility and all)

1. cassandra_keyspaces is undefined --> all keyspaces are being repaired
2. cassandra_keyspaces is defined && len(cassandra_keyspaces) == 0 --> repair no keyspace / don't run repair
3. cassandra_keyspaces is defined && len(cassandra_keyspaces) > 0 --> repair the listed keyspaces

[jschaul]

Would it make sense to allow 3 cases so that cassandra_keyspaces can be kept? (backward compatibility and all)

1. `cassandra_keyspaces is undefined` --> all keyspaces are being repaired

2. `cassandra_keyspaces is defined && len(cassandra_keyspaces) == 0` --> repair no keyspace / don't run repair

3. `cassandra_keyspaces is defined && len(cassandra_keyspaces) > 0` --> repair the listed keyspaces

That doesn't sound like a good design. You always want to have repairs running for all of your keyspaces. You get data loss otherwise. The only use case I can see where you do not want to run repairs via cron jobs for all of your keyspaces is if you have some other tool that runs repairs for you. We don't have any such tool at this point. But if we had, then it would make sense to have a variables that's explicit, like e.g. cassandra_repairs_enabled: false. Doing conditional logic based on length of lists is terrible.
I don't think we need any 'backwards compatibility' here - we want to change the defaults to be "production settings by default', so that changes the behaviour - but in a desired fashion. A note in the changelog should suffice.

One more thought @tiago-loureiro : what about the 'system' keyspace? This maybe changes behaviour slightly (was it never being repaired before? Or done implicitly? Or it's per-node so there cannot be any data inconsistency?)

[tiago-loureiro]

Would it make sense to allow 3 cases so that cassandra_keyspaces can be kept? (backward compatibility and all)

1. cassandra_keyspaces is undefined --> all keyspaces are being repaired
2. cassandra_keyspaces is defined && len(cassandra_keyspaces) == 0 --> repair no keyspace / don't run repair
3. cassandra_keyspaces is defined && len(cassandra_keyspaces) > 0 --> repair the listed keyspaces

I'd prefer to remove any config unless necessary. Not running any repairs is pretty dangerous and I don't see a good reason to avoid running on any keyspace given that the tool seems to be harmless on keyspaces that don't need repairing.

One more thought @tiago-loureiro : what about the 'system' keyspace? This maybe changes behaviour slightly (was it never being repaired before? Or done implicitly? Or it's per-node so there cannot be any data inconsistency?)

All system_* keyspaces are local and have a replication factor 1 and don't really need to have repairs run on them; the repair logic is smart enough to simply ignore it, e.g.,: Replication factor is 1. No repair is needed for keyspace 'system_auth'