kfaraz commented on code in PR #18855:
URL: https://github.com/apache/druid/pull/18855#discussion_r2641815034
##########
processing/src/main/java/org/apache/druid/segment/incremental/RowIngestionMetersTotals.java:
##########
@@ -109,7 +156,19 @@ public String toString()
", processedBytes=" + processedBytes +
", processedWithError=" + processedWithError +
", thrownAway=" + thrownAway +
+ ", thrownAwayByReason=" + thrownAwayByReason +
", unparseable=" + unparseable +
'}';
}
+
+ /**
+ * For backwards compatibility, key by {@link InputRowFilterResult} in case
of lack of thrownAwayByReason input during rolling Druid upgrades.
+ * This can occur when tasks running on older Druid versions return ingest
statistic payloads to an overlord running on a newer Druid version.
+ */
+ private static Map<String, Long>
getBackwardsCompatibleThrownAwayByReason(long thrownAway)
+ {
+ Map<String, Long> results = InputRowFilterResult.buildRejectedCounterMap();
+ results.put(InputRowFilterResult.UNKNOWN.getReason(), thrownAway);
+ return results;
Review Comment:
In the future, we may have a large number of reasons. It wouldn't make sense
to have a zero entry for all of them in the payload or to emit a zero metric
for all of them. It would just bloat up the payload.
> Additionally, this is already the pattern with the rest of the counters
(processed, etc.)
It makes sense for `processed` to always be reported (even if zero), since
it indicates that processing was actually attempted. It is a "success" metric
in the sense that the absence of the metric also indicates some erratic
behaviour. This is generally true for gauge metrics too.
`thrownAway` on the other hand is more of an "error" count metric that need
not be emitted if nothing was thrown away. This metric is typically useful only
when we are looking for the reason of why certain records didn't make it into
the system, or _did_ make it into the system even when we expected them to get
discarded.
In both the cases, the absence of the metric is equivalent to a zero value
as both indicate that nothing was thrown away. Downstream applications could
treat absence/null the same as zero in this case.
> (e.g. did the event not exist, did the filter reason not exist, did the
task not check for that reason?)
Yes, the `thrownAway` metric is not equipped to answer these questions and
should not try to either. By populating the values for all possible reasons as
0, we are still not answering these questions.
Same as any other error metric, the absence of the metric should simply
indicate that the error did not occur.
Any further reason should be determined with the help of other success
metrics (`processed` etc).
--
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]
