bube opened a new pull request, #9158:
URL: https://github.com/apache/paimon/pull/9158
### Purpose
Closes #9157.
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. In our case a shaded jar
had dropped the `META-INF/services` entry for
`IcebergRESTMetadataCommitterFactory`.
This logs a `WARN` instead:
```
No IcebergMetadataCommitterFactory for
'metadata.iceberg.storage=rest-catalog' found on
the classpath, so table mydb.t 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/org.apache.paimon.factories.Factory entry survived
shading.
Cause: Could not find any factories that implement
'org.apache.paimon.iceberg.IcebergMetadataCommitterFactory' in the classpath.
```
`disabled`, `table-location` and `hadoop-catalog` have no committer factory
by design and must stay quiet. `IcebergOptions.StorageType` now answers that
question instead of a switch in the caller:
```java
HIVE_CATALOG("hive-catalog", "...", true),
REST_CATALOG("rest-catalog", "...", true);
```
It is a required constructor argument, so a new storage type cannot be added
without stating its own answer. A `committerFactoryIdentifier()` accessor
replaces the incidental use of `toString()` as the SPI lookup key. Both
factories already derived their identifier from this enum, so this only names
the contract.
Still a warning and not a failure, so jobs in this state will not start
erroring on upgrade.
### Tests
No new test. Reproducing a missing SPI registration inside a module that has
one needs classloader manipulation that would test the harness more than the
code. Verified by running it across every `StorageType`:
| `metadata.iceberg.storage` | warns? |
|---|---|
| `table-location` | no |
| `hadoop-catalog` | no |
| `hive-catalog` | yes |
| `rest-catalog` | yes |
Existing suites pass on JDK 11:
- `paimon-core`: `IcebergCommitCallbackTest` 28/28,
`IcebergCompatibilityTest` 40/40
- `paimon-iceberg`: 30 unit, 4 IT including
`IcebergRestMetadataCommitterITCase`
- `paimon-hive-catalog`: 62 unit, 14 IT
spotless, checkstyle, enforcer and rat are clean.
### API and Format
No format change. `StorageType` gains two public accessors. Constants,
values and descriptions are unchanged, so no config or generated-docs change.
### Documentation
None needed.
--
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]
