rdblue commented on code in PR #7337:
URL: https://github.com/apache/iceberg/pull/7337#discussion_r1170602203
##########
core/src/main/java/org/apache/iceberg/TableScanContext.java:
##########
@@ -353,17 +352,29 @@ TableScanContext planWith(ExecutorService executor) {
toSnapshotId,
executor,
fromSnapshotInclusive,
- metricsReporters);
+ metricsReporter);
}
- Collection<MetricsReporter> metricsReporters() {
- return metricsReporters;
+ MetricsReporter metricsReporter() {
+ return metricsReporter;
}
TableScanContext reportWith(MetricsReporter reporter) {
- ImmutableList.Builder<MetricsReporter> builder = ImmutableList.builder();
- builder.addAll(metricsReporters);
- builder.add(reporter);
+ CompositeMetricsReporter compositeReporter;
Review Comment:
Why does this always create a composite reporter? That seems strange.
I would expect the metrics reporter to initially be null. When this is
called the first time, a context with a non-null reporter is created. When it
is subsequently called, a composite reporter is used. If one is already in the
context, then a new one is created with all of the old reporters plus the new
reporter.
We also need to think about deduplication. What happens if the logging
reporter is added by two separate code paths? How does the composite reporter
detect this?
--
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]
