Iceberg HMS: CREATE TABLE failure handling can delete the metadata of a table a later commit already built on
Trino version
483 and later (master)
Please describe the bug
AbstractMetastoreTableOperations.commitNewTable verifies the outcome of a failed createTable since #29973: it re-reads the table and treats any metadata_location other than the one it just wrote as proof that another writer owns the name, then deletes the new metadata file and throws CreateTableException (a CleanableFailure, so the manifest list and manifests are removed as well).
That is wrong when the create was applied and another commit already built on it before the client observed the failure. The Thrift client retries create_table after a read timeout; if the first attempt was applied, the retry fails with AlreadyExistsException (or a duplicate-key MetaException when the two attempts overlap inside HMS), and anything that committed on the new table in the meantime has moved metadata_location past our file. The current metadata then descends from the file this operation wrote, and deleting that file plus its manifest list and manifests corrupts the table: reads fail, or, when the create carried no data and the later commit added a snapshot, the first snapshot can no longer be read or expired. That is the outcome #29973 set out to prevent.
We hit the triggering sequence in production (on a version before #29973): a create_table RPC timed out while the metastore was slow on HDFS, the Thrift client retried it, and the retry got a duplicate-key error after the first attempt had committed. The window for a concurrent commit is the retry's remaining latency, and under the same slowness that caused the timeout that is seconds rather than milliseconds; a second pipeline instance running CREATE TABLE IF NOT EXISTS followed by INSERT lands its commit right in it.
Related: when the cleanup deleteFile itself throws, its exception replaces the create failure, so the user sees a file system error instead of the reason the create failed, and CreateTableException is never thrown, so Iceberg skips its own cleanup.
Proposal (PR to follow):
- When the metastore points at other metadata, read it and compare
TableMetadata.uuid()with the UUID this operation assigned. The UUID is set once bynewTableMetadataand preserved by every later commit (TableMetadata.Builder(base),buildReplacement), so a match means the table descends from our create and no foreign create can produce it; it is not bounded bywrite.metadata.previous-versions-maxthe way a metadata-log lookup would be. - Log a failed cleanup and throw
CreateTableExceptionregardless, like Iceberg'sHiveOperationsBase.cleanupMetadata.
Source: trinodb/trino