From cbed40dd41df1afe9b7924f195cabca6e0f111e9 Mon Sep 17 00:00:00 2001 From: Andreas Bube Date: Mon, 10 Aug 2026 21:53:02 +0200 Subject: [PATCH] [iceberg] Warn when the Iceberg metadata committer factory is missing from the classpath When metadata.iceberg.storage requests a sync to an external catalog but the committer factory cannot be discovered, Paimon skipped the sync silently. IcebergCommitCallback caught the FactoryException and discarded it, leaving metadataCommitter null so commitToExternalCatalog returned early on every commit. Commits kept succeeding and metadata files kept being written, so the job looked healthy while nothing reached the catalog. Log a WARN in that case instead. The disabled, table-location and hadoop-catalog storage types have no committer factory by design and must stay quiet, so StorageType now carries whether it needs one as a required constructor argument rather than a switch in the caller. A new committerFactoryIdentifier() accessor replaces the incidental use of toString() as the SPI lookup key. Co-Authored-By: Claude Opus 5 --- .../paimon/iceberg/IcebergCommitCallback.java | 19 ++++++++++-- .../apache/paimon/iceberg/IcebergOptions.java | 31 +++++++++++++++---- .../IcebergHiveMetadataCommitterFactory.java | 2 +- .../IcebergRESTMetadataCommitterFactory.java | 2 +- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergCommitCallback.java b/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergCommitCallback.java index 4b6776d3396b..54180fe3e07e 100644 --- a/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergCommitCallback.java +++ b/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergCommitCallback.java @@ -23,6 +23,7 @@ import org.apache.paimon.data.BinaryRow; import org.apache.paimon.data.GenericArray; import org.apache.paimon.data.GenericRow; +import org.apache.paimon.factories.Factory; import org.apache.paimon.factories.FactoryException; import org.apache.paimon.factories.FactoryUtil; import org.apache.paimon.fs.FileStatus; @@ -165,9 +166,23 @@ public IcebergCommitCallback(FileStoreTable table, String commitUser) { FactoryUtil.discoverFactory( IcebergCommitCallback.class.getClassLoader(), IcebergMetadataCommitterFactory.class, - storageType.toString()); - } catch (FactoryException ignore) { + storageType.committerFactoryIdentifier()); + } catch (FactoryException e) { metadataCommitterFactory = null; + // storage types without a committer have no factory by design, so a miss is expected + if (storageType.requiresMetadataCommitter()) { + LOG.warn( + "No IcebergMetadataCommitterFactory for '{}={}' found on the classpath, so " + + "table {} will not be synced to the external catalog (commits and " + + "metadata files are unaffected). Check that the module providing it " + + "is deployed and that its META-INF/services/{} entry survived " + + "shading. Cause: {}", + IcebergOptions.METADATA_ICEBERG_STORAGE.key(), + storageType, + table.fullName(), + Factory.class.getName(), + e.getMessage()); + } } this.metadataCommitter = metadataCommitterFactory == null ? null : metadataCommitterFactory.create(table); diff --git a/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergOptions.java b/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergOptions.java index 530acd85eb4f..c28716dc25a8 100644 --- a/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergOptions.java +++ b/paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergOptions.java @@ -198,27 +198,46 @@ public int previousVersionsMax() { /** Where to store Iceberg metadata. */ public enum StorageType implements DescribedEnum { - DISABLED("disabled", "Disable Iceberg compatibility support."), - TABLE_LOCATION("table-location", "Store Iceberg metadata in each table's directory."), + DISABLED("disabled", "Disable Iceberg compatibility support.", false), + TABLE_LOCATION( + "table-location", "Store Iceberg metadata in each table's directory.", false), HADOOP_CATALOG( "hadoop-catalog", "Store Iceberg metadata in a separate directory. " - + "This directory can be specified as the warehouse directory of an Iceberg Hadoop catalog."), + + "This directory can be specified as the warehouse directory of an Iceberg Hadoop catalog.", + false), HIVE_CATALOG( "hive-catalog", "Not only store Iceberg metadata like hadoop-catalog, " - + "but also create Iceberg external table in Hive."), + + "but also create Iceberg external table in Hive.", + true), REST_CATALOG( "rest-catalog", "Store Iceberg metadata in a REST catalog. " - + "This allows integration with Iceberg REST catalog services."); + + "This allows integration with Iceberg REST catalog services.", + true); private final String value; private final String description; + private final boolean requiresMetadataCommitter; - StorageType(String value, String description) { + StorageType(String value, String description, boolean requiresMetadataCommitter) { this.value = value; this.description = description; + this.requiresMetadataCommitter = requiresMetadataCommitter; + } + + /** Whether this storage type syncs metadata to an external catalog via a committer. */ + public boolean requiresMetadataCommitter() { + return requiresMetadataCommitter; + } + + /** + * Identifier this storage type's {@code IcebergMetadataCommitterFactory} is registered + * under. + */ + public String committerFactoryIdentifier() { + return value; } @Override diff --git a/paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/iceberg/IcebergHiveMetadataCommitterFactory.java b/paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/iceberg/IcebergHiveMetadataCommitterFactory.java index 2ae279d3d7af..d858852208d3 100644 --- a/paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/iceberg/IcebergHiveMetadataCommitterFactory.java +++ b/paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/iceberg/IcebergHiveMetadataCommitterFactory.java @@ -25,7 +25,7 @@ public class IcebergHiveMetadataCommitterFactory implements IcebergMetadataCommi @Override public String identifier() { - return IcebergOptions.StorageType.HIVE_CATALOG.toString(); + return IcebergOptions.StorageType.HIVE_CATALOG.committerFactoryIdentifier(); } @Override diff --git a/paimon-iceberg/src/main/java/org/apache/paimon/iceberg/IcebergRESTMetadataCommitterFactory.java b/paimon-iceberg/src/main/java/org/apache/paimon/iceberg/IcebergRESTMetadataCommitterFactory.java index 8b2bf719a087..ee708d6c9493 100644 --- a/paimon-iceberg/src/main/java/org/apache/paimon/iceberg/IcebergRESTMetadataCommitterFactory.java +++ b/paimon-iceberg/src/main/java/org/apache/paimon/iceberg/IcebergRESTMetadataCommitterFactory.java @@ -24,7 +24,7 @@ public class IcebergRESTMetadataCommitterFactory implements IcebergMetadataCommitterFactory { @Override public String identifier() { - return IcebergOptions.StorageType.REST_CATALOG.toString(); + return IcebergOptions.StorageType.REST_CATALOG.committerFactoryIdentifier(); } @Override