gortiz commented on code in PR #13035:
URL: https://github.com/apache/pinot/pull/13035#discussion_r1628906266
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/MultiStageQueryStats.java:
##########
@@ -284,14 +284,18 @@ public void mergeUpstream(List<ByteBuffer> otherStats) {
myStats.merge(dis);
}
} catch (IOException ex) {
- LOGGER.warn("Error deserializing stats on stage {}. Considering the
new stats empty", i, ex);
+ LOGGER.warn("Error deserializing stats on stage " + i + ".
Considering the new stats empty", ex);
} catch (IllegalArgumentException | IllegalStateException ex) {
- LOGGER.warn("Error merging stats on stage {}. Ignoring the new
stats", i, ex);
+ LOGGER.warn("Error merging stats on stage " + i + ". Ignoring the
new stats", ex);
}
}
}
}
+ public List<StageStats.Closed> getClosedStats() {
+ return Collections.unmodifiableList(_closedStats);
Review Comment:
A unmodifiable list is very light (just an indirection) and TBH I prefer to
be a bit paranoic here so we don't end up modifying it in the future, which
would be difficult to catch.
This method is only called once per query on the broker. I don't think
creating a single small object (that can be probably scalar replaced) would be
problematic.
--
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]
