devmadhuu commented on code in PR #10384:
URL: https://github.com/apache/ozone/pull/10384#discussion_r3378078337
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerSyncHelper.java:
##########
@@ -390,6 +420,39 @@ private boolean syncDeletedContainers() {
} catch (Exception e) {
LOG.error("DELETED sync: unexpected error.", e);
return false;
+ } finally {
+ updateContainerSyncDuration(HddsProtos.LifeCycleState.DELETED,
+ Time.monotonicNow() - startTime);
+ }
+ }
+
+ private void updateDeletedContainerCountDrift() {
+ if (containerSyncMetrics == null) {
+ return;
+ }
+ try {
+ long total = scmServiceProvider.getContainerCount(
+ HddsProtos.LifeCycleState.DELETED);
+ updateContainerCountDrift(HddsProtos.LifeCycleState.DELETED, total);
+ } catch (Exception e) {
+ LOG.warn("DELETED sync: unable to update pre-sync count drift metric.",
e);
Review Comment:
- Using -1 is not safe for this metric because drift is signed:
drift = SCM count - Recon count
- -1 is a valid real drift value, meaning Recon has one more container in
that state than SCM.
- Long.MIN_VALUE is less likely to create problem, but it would still mix
“measurement failed” into a numeric drift gauge and complicate
dashboards/alerts.
I checked nearby Ozone/Recon patterns:
- There are -1 sentinel usages for API/UI count fields and task status
style values.
- I did not find a valid usage for using -1 or Long.MIN_VALUE as a
Metrics2 gauge for a signed drift metric.
- Recon metrics commonly use explicit failure counters for failure
visibility.
My preference is to keep the last valid drift value on count-read failure
and rely on the warning log, or add a separate failure counter if we want
metric-visible failure tracking. In this case WARN log is sufficient IMO.
--
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]
