rdblue commented on code in PR #16155:
URL: https://github.com/apache/iceberg/pull/16155#discussion_r3174367529


##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/IcebergStreamWriterMetrics.java:
##########
@@ -97,4 +108,72 @@ public Counter getFlushedDataFiles() {
   public Counter getFlushedDeleteFiles() {
     return flushedDeleteFiles;
   }
+
+  @VisibleForTesting
+  @Nullable
+  Histogram dataFilesSizeHistogram() {
+    return dataFilesSizeHistogram;
+  }
+
+  @VisibleForTesting
+  @Nullable
+  Histogram deleteFilesSizeHistogram() {
+    return deleteFilesSizeHistogram;
+  }
+
+  /**
+   * Checks whether the Dropwizard-based histogram wrapper provided through 
Flink's optional
+   * flink-metrics-dropwizard dependency is available.
+   */
+  @SuppressWarnings("CatchBlockLogException")
+  private static Histogram loadHistogramIfAvailable(
+      MetricGroup group, String name, int reservoirSize, ClassLoader 
classLoader) {

Review Comment:
   Why does this pass the group and return the histogram? Doesn't this result 
in a side effect on the group? It seems like this should just create the 
histogram and the caller should add it to the group. That way this has no 
side-effects and 1 fewer arguments.



-- 
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]

Reply via email to