dcapwell commented on code in PR #2832:
URL: https://github.com/apache/cassandra/pull/2832#discussion_r1369311711
##########
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:
> We have the thread-safety guarantees because Verb messages are never
processed in parallel, right?
we can process multiple repairs in parallel, so that isn't true.
> computeIfAbsent doesn't look like a safe option for synchronizedMap
because it's still not an atomic operation
`java.util.Collections.SynchronizedMap#computeIfAbsent` is the following on
JDK 11
```
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction) {
synchronized (mutex) {return m.computeIfAbsent(key,
mappingFunction);}
}
```
@Mmuzaf where did you see it not use the lock? This is thread safe on JDK
11...
> yeah, I think ConcurrentHashMap is a better choice
This is thread safe on JDK 11 and should have less memory, so the fact that
CHM is faster with contention isn't as valuable to me as having less memory.
--
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]
