[GitHub] [iceberg] nastra commented on a diff in pull request #6919: Add HasScan metrics interface

["nastra (via GitHub)" <gi...@apache.org>]

nastra commented on code in PR #6919:
URL: https://github.com/apache/iceberg/pull/6919#discussion_r1115974741


##########
api/src/main/java/org/apache/iceberg/BatchScanAdapter.java:
##########
@@ -140,4 +140,8 @@ public int splitLookback() {
   public long splitOpenFileCost() {
     return scan.splitOpenFileCost();
   }
+
+  public Scan getScanDelegate() {

Review Comment:
   it's not clear why is this change is needed. Could you elaborate please?



##########
core/src/main/java/org/apache/iceberg/SnapshotScan.java:
##########
@@ -66,7 +66,8 @@ protected Long snapshotId() {
 
   protected abstract CloseableIterable<T> doPlanFiles();
 
-  protected ScanMetrics scanMetrics() {
+  @Override
+  public ScanMetrics scanMetrics() {

Review Comment:
   `ScanMetrics` shouldn't be exposed and should be treated as internal information as they can change while the scan is going on. What you'd rather want to do is to look at the `ScanReport`, which will be created once a scan is complete.
   
   So in your case what would make sense is to have a custom `MetricsReporter` that receives the `ScanReport` when a scan is done, which you can then use to hook it up to the Spark UI.
   In order to 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org