Copilot commented on code in PR #3740:
URL: https://github.com/apache/celeborn/pull/3740#discussion_r3726344308
##########
common/src/main/scala/org/apache/celeborn/common/metrics/source/AbstractSource.scala:
##########
@@ -186,16 +205,20 @@ abstract class AbstractSource(conf: CelebornConf, role:
String)
})
}
- def addCounter(name: String): Unit = addCounter(name, Map.empty[String,
String])
+ def addCounter(name: String): NamedCounter = addCounter(name,
Map.empty[String, String])
- def addCounter(name: String, labels: Map[String, String]): Unit = {
+ def addCounter(name: String, labels: Map[String, String]): NamedCounter = {
val metricNameWithLabel = metricNameWithCustomizedLabels(name, labels)
- namedCounters.putIfAbsent(
+ // Atomically get-or-create so the returned NamedCounter is guaranteed to
be the instance
+ // currently resident in namedCounters. Callers must never
re-.get(metricNameWithLabel)
+ // afterwards, since a concurrent removeCounter could have removed it in
the meantime.
+ namedCounters.computeIfAbsent(
Review Comment:
`addCounter` now relies on `ConcurrentHashMap.computeIfAbsent` with a Scala
lambda. With Scala 2.11 cross-build enabled in this repo, Scala lambdas are not
SAM-converted to `java.util.function.Function` (no `-Xexperimental` /
java8-compat converters in build), so this is likely to break compilation for
2.11. Prefer a Scala-2.11-compatible get-or-create implementation (e.g.,
`putIfAbsent` and return the resident instance).
--
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]
