[DNM] Flags for disabling read repairs #641
Conversation
rhuffy
changed the base branch from
palantir-cassandra-2.2.18
to
rh/hotfix-coerce-CL-repair
| // wait for the repair writes to be acknowledged, to minimize impact on any replica that's | ||
| // behind on writes in case the out-of-sync row is read multiple times in quick succession | ||
| FBUtilities.waitOnFutures(resolver.repairResults, DatabaseDescriptor.getWriteRpcTimeout()); | ||
| } |
There was a problem hiding this comment.
If that flag is set, do we need to disable waiting on repair results in getRangeSlice as well?
There was a problem hiding this comment.
Probably worth updating the metrics marking as well ReadRepairMetrics.repairedBlocking
| @@ -109,6 +110,11 @@ public Row resolve() throws DigestMismatchException | |||
| */ | |||
| public static List<AsyncOneResponse> scheduleRepairs(ColumnFamily resolved, String keyspaceName, DecoratedKey key, List<ColumnFamily> versions, List<InetAddress> endpoints) | |||
There was a problem hiding this comment.
I think there are other parts of the code that do async read repairs along the write path:
. But those are async as far as I can tell and don't seem to be leveraged in the read path. So maybe not worth tackling for nowThere was a problem hiding this comment.
Yeah, I'm not convinced that disabling async read repairs is even desirable at this point
There was a problem hiding this comment.
From what I can tell, I think this actually goes back into RowDataResolver::resolve code path and from a quick lookup in the code MessagingService.Verb.READ_REPAIR is only leveraged in scheduleRepairs. So we would effectively be disabling all read repairs on reads or writes (which makes me lean towards you and Lyuben's suggestion to be incremental more, considering we seem to already be throwing around a bunch of async read repairs when we write)
Co-authored-by: Daniel Guo <[email protected]>
| FBUtilities.waitOnFutures(repairResponses, DatabaseDescriptor.getWriteRpcTimeout()); | ||
| if (!DatabaseDescriptor.getDisableBlockOnReadRepair()) | ||
| { | ||
| FBUtilities.waitOnFutures(repairResponses, DatabaseDescriptor.getWriteRpcTimeout()); | ||
| } |
There was a problem hiding this comment.
here's the addition in getRangeSlice
| GetDisableReadRepairMutation.class, | ||
| SetDisableReadRepairMutation.class, | ||
| GetDisableBlockOnReadRepair.class, | ||
| SetDisableBlockOnReadRepair.class |
There was a problem hiding this comment.
supporting in nodetool
| if (command.isDigestQuery()) | ||
| { | ||
| Keyspace.open(command.ksName).getColumnFamilyStore(command.cfName).metric.digestReads.mark(); | ||
| } | ||
| else | ||
| { | ||
| Keyspace.open(command.ksName).getColumnFamilyStore(command.cfName).metric.dataReads.mark(); | ||
| } |
There was a problem hiding this comment.
metrics on data vs. digest reads
| @@ -1450,6 +1450,7 @@ private static List<Row> fetchRows(List<ReadCommand> initialCommands, Consistenc | |||
| try | |||
| { | |||
| Row row = exec.get(); | |||
| Keyspace.open(exec.command.ksName).getColumnFamilyStore(exec.command.cfName).metric.avoidedReadRepairs.mark(); | |||
There was a problem hiding this comment.
For future ref, this metric is placed here because exec.get() will throw a DigestMismatchException if values do not agree
| blockingReadRepairs = Metrics.meter(factory.createMetricName("BlockingReadRepairs")); | ||
| backgroundReadRepairs = Metrics.meter(factory.createMetricName("BackgroundReadRepairs")); | ||
| attemptedReadRepairs = Metrics.meter(factory.createMetricName("AttemptedReadRepairs")); | ||
| digestReads = Metrics.meter(factory.createMetricName("DigestReads")); | ||
| dataReads = Metrics.meter(factory.createMetricName("DataReads")); |
There was a problem hiding this comment.
Based on #634 and #630
This PR adds two new config options to
cassandra.yaml, and their respectivenodetoolcommands for checking/setting at runtime.disable_block_on_read_repair- this coordinator will read-repair as normal, but will not block on ACKs forREAD_REPAIRverbs before returning to the client. The intention is to reduce the latency of read-repairs, at the cost of read-repairing multiple times if the same cell is read repeatedly.disable_read_repair_mutation- this coordinator will still resolve a full read in the case of digest mismatches, but will not sendREAD_REPAIRmutation verbs to any replicas. The intention is to reduce the write load on replicas.