You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2026/10/13 06:53:56 UTC

(doris) branch master updated: [fix](iceberg) Keep the table's metrics reporter in the Kerberos scan wrapper (#68966)

This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new c3ea40a2018 [fix](iceberg) Keep the table's metrics reporter in the Kerberos scan wrapper (#68966)
c3ea40a2018 is described below

commit c3ea40a20188c2047a5d24cf91c986654da8ef61
Author: Mingyu Chen (Rayner) <mo...@gmail.com>
AuthorDate: Tue Oct 13 14:53:47 2026 +0800

    [fix](iceberg) Keep the table's metrics reporter in the Kerberos scan wrapper (#68966)
    
    ### What problem does this PR solve?
    
    Issue Number: None
    
    Related PR: #68965
    
    Problem Summary:
    
    **In short.** On a Kerberos catalog, every Iceberg scan runs on a
    wrapper table that drops the table's metrics reporter, so the catalog's
    reporter never receives a scan report from such a catalog. This PR
    passes the table's reporter to the wrapper. It is not a branch-4.x port;
    it came up next to #68965, which fixes the same loss for
    historical-schema scans.
    
    **Background.** Iceberg reports each scan's metrics through the
    `MetricsReporter` that `BaseTable` holds; a catalog can configure one,
    for example through `metrics-reporter-impl`. On a Kerberos catalog,
    `IcebergScanPlanProvider.wrapTableForScan` wraps every scanned table in
    a new `BaseTable` whose `FileIO` runs inside the plugin's doAs, so that
    Iceberg's worker threads read files with the right credentials.
    
    **The problem, and what it cost.**
    
    | Table | Reporter of the scanned table on master | With this PR |
    |---|---|---|
    | Kerberos catalog, `wrapTableForScan` | `LoggingMetricsReporter` (the
    two-argument `BaseTable` constructor's default), for current and
    historical schemas alike | the table's reporter |
    | Kerberos catalog, `wrapTableForProcedure` | the table's reporter |
    unchanged |
    | Snapshot tables of `IcebergTableCache` and `IcebergStatementScope` |
    the table's reporter | unchanged |
    
    Query results were not affected.
    
    **How this PR fixes it.** `wrapTableForScan` builds the wrapper with the
    three-argument constructor and the table's reporter, as
    `wrapTableForProcedure` and the snapshot tables already do.
    
    **Results.** The new unit test wraps a table that carries a custom
    reporter under a Kerberos context: before the fix the wrapper holds
    `LoggingMetricsReporter`, after it the table's reporter. All
    fe-connector-iceberg tests pass (1525).
---
 .../connector/iceberg/IcebergScanPlanProvider.java |  6 ++++--
 .../IcebergScanPlanProviderKerberosScanIoTest.java | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
index 09b77b83bb4..05055535e72 100644
--- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
+++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java
@@ -3193,9 +3193,11 @@ public class IcebergScanPlanProvider implements ConnectorScanPlanProvider {
         if (auth == null) {
             return table;
         }
-        TableOperations rawOps = ((BaseTable) table).operations();
+        BaseTable baseTable = (BaseTable) table;
+        TableOperations rawOps = baseTable.operations();
+        // Keep the table's metrics reporter: every scan of a Kerberos catalog runs on this wrapper.
         return new BaseTable(new IcebergAuthenticatedTableOperations(
-                rawOps, new IcebergAuthenticatedFileIO(rawOps.io(), auth)), table.name());
+                rawOps, new IcebergAuthenticatedFileIO(rawOps.io(), auth)), table.name(), baseTable.reporter());
     }
 
     /**
diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderKerberosScanIoTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderKerberosScanIoTest.java
index 66c5c05bc09..7c89496823f 100644
--- a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderKerberosScanIoTest.java
+++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderKerberosScanIoTest.java
@@ -31,6 +31,7 @@ import org.apache.iceberg.Table;
 import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.inmemory.InMemoryCatalog;
+import org.apache.iceberg.metrics.MetricsReporter;
 import org.apache.iceberg.types.Types;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -173,6 +174,27 @@ public class IcebergScanPlanProviderKerberosScanIoTest {
                         + "FileSystem for later newStream() on any thread (incl. iceberg's worker pool)");
     }
 
+    @Test
+    public void wrapTableForScanKeepsTheTableMetricsReporter() {
+        Table table = oneFileTable();
+        MetricsReporter catalogReporter = report -> { };
+        ((BaseTable) table).combineMetricsReporter(catalogReporter);
+        RecordingAuthenticator auth = new RecordingAuthenticator();
+        TcclPinningConnectorContext context = new TcclPinningConnectorContext(
+                new RecordingConnectorContext(), getClass().getClassLoader(), () -> auth);
+        IcebergScanPlanProvider provider =
+                new IcebergScanPlanProvider(IcebergCatalogProperties.of(Collections.emptyMap()), opsReturning(table), context);
+
+        Table wrapped = provider.wrapTableForScan(table);
+
+        // WHY: every scan of a Kerberos catalog runs on the wrapper, so it must carry the table's metrics reporter
+        // like wrapTableForProcedure and the snapshot tables do; the two-argument BaseTable constructor falls back
+        // to LoggingMetricsReporter and the catalog's reporter never sees a scan.
+        // MUTATION: building the wrapper without the table's reporter -> red.
+        Assertions.assertNotSame(table, wrapped);
+        Assertions.assertSame(((BaseTable) table).reporter(), ((BaseTable) wrapped).reporter());
+    }
+
     @Test
     public void plainContextWrapIsIdentityPassThrough() {
         Table table = oneFileTable();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org