xiangfu0 commented on code in PR #19944:
URL: https://github.com/apache/pinot/pull/19944#discussion_r4343872056
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/PinotLogicalQueryPlanner.java:
##########
@@ -161,4 +175,49 @@ private static PlanFragment planNodeToPlanFragment(
return new PlanFragment(0, rootReceiveNode, List.of(planFragment1));
}
+
+ /// Emits the spool meters and, when `spoolsFeedback` is set, attaches a
`spools` entry to the response metadata.
+ ///
+ /// The entry is attached even when spools are off, so that a user who asks
why nothing was shared learns the mode.
+ private static void reportSpools(SpoolMode spoolMode, boolean spoolsFeedback,
+ List<SpoolSubstitution> substitutions) {
+ int applied = 0;
+ int rejectedByGuard = 0;
+ for (SpoolSubstitution substitution : substitutions) {
+ if (substitution.outcome() == SpoolSubstitution.Outcome.APPLIED) {
+ applied++;
+ } else if (substitution.outcome() ==
SpoolSubstitution.Outcome.REJECTED_BY_GUARD) {
+ rejectedByGuard++;
+ }
+ }
+ BrokerMetrics brokerMetrics = BrokerMetrics.get();
+ if (applied > 0) {
+ brokerMetrics.addMeteredGlobalValue(BrokerMeter.SPOOLS_APPLIED, applied);
Review Comment:
MINOR: as the PR body already notes, this path runs for EXPLAIN too, so
`SPOOLS_APPLIED`/`SPOOLS_REJECTED_BY_GUARD` will be incremented by EXPLAIN
queries. That matches the existing `JOIN_COUNT` behavior, so it's consistent —
just flagging so dashboards built on these meters don't read EXPLAIN traffic as
real spool activity.
--
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]
