SteNicholas commented on code in PR #3740:
URL: https://github.com/apache/celeborn/pull/3740#discussion_r3802876922
##########
client/src/main/scala/org/apache/celeborn/client/ApplicationHeartbeater.scala:
##########
@@ -85,10 +99,14 @@ class ApplicationHeartbeater(
tmpApplicationFallbackCounts.asJava,
workerStatusTracker.getNeedCheckedWorkers().toList.asJava,
ZERO_UUID,
- true)
+ true,
+ if (appMetricLabels.isEmpty)
java.util.Collections.emptyMap[String, ClientMetric]()
+ else clientMetrics(),
+ appMetricLabels)
val response = requestHeartbeat(appHeartbeat)
if (response.statusCode == StatusCode.SUCCESS) {
logDebug("Successfully send app heartbeat.")
+ commitClientMetrics()
Review Comment:
Gauges-only is a reasonable way to avoid the ambiguous-ack problem in this
PR, provided all counter collection, wire types, and master-side counter
aggregation are removed from this change. The current head still includes the
eight counters, so this P1 remains unresolved. Before landing a gauges-only
version, please also address the shared-label gauge semantics noted in
https://github.com/apache/celeborn/pull/3740#discussion_r3802873927; otherwise
each series is merely the last heartbeating application value.
--
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]
