frankgh commented on code in PR #2832:
URL: https://github.com/apache/cassandra/pull/2832#discussion_r1368143725
##########
src/java/org/apache/cassandra/repair/messages/RepairMessage.java:
##########
@@ -165,13 +171,33 @@ public void onFailure(InetAddressAndPort from,
RequestFailureReason failureReaso
backoff.computeWaitTime(attempt), backoff.unit());
return;
}
+ maybeRecordRetry(false);
finalCallback.onFailure(from, failureReason);
return;
default:
throw new AssertionError("Unknown error handler: " +
allowed);
}
}
+ private void maybeRecordRetry(boolean succcess)
+ {
+ if (attempt > 0) // if greater than 0 we know a retry
happened...
+ {
+ // we don't know what the prefix kind is... so use NONE...
this impacts logPrefix as it will cause us to use "repair" rather than "preview
repair" which may not be correct... but close enough...
+ String prefix =
PreviewKind.NONE.logPrefix(request.parentRepairSession());
+ RepairMetrics.retry(verb, attempt);
+ if (succcess)
+ {
+ noSpam.info("{} Retry of repair verb " + verb + " was
success after {} attempts", prefix, attempt);
Review Comment:
NIT
```suggestion
noSpam.info("{} Retry of repair verb " + verb + "
was successful after {} attempts", prefix, attempt);
```
##########
src/java/org/apache/cassandra/metrics/RepairMetrics.java:
##########
@@ -18,17 +18,41 @@
package org.apache.cassandra.metrics;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.Map;
+
import com.codahale.metrics.Counter;
+import com.codahale.metrics.Histogram;
+import org.apache.cassandra.net.Verb;
import static org.apache.cassandra.metrics.CassandraMetricsRegistry.Metrics;
public class RepairMetrics
{
public static final String TYPE_NAME = "Repair";
public static final Counter previewFailures =
Metrics.counter(DefaultNameFactory.createMetricName(TYPE_NAME,
"PreviewFailures", null));
+ private static final Histogram retries =
Metrics.histogram(DefaultNameFactory.createMetricName(TYPE_NAME, "Retries",
null), false);
+ private static final Map<Verb, Histogram> retriesByVerb =
Collections.synchronizedMap(new EnumMap<>(Verb.class));
Review Comment:
yeah, I think `ConcurrentHashMap` is a better choice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
