wombatu-kun opened a new pull request, #16943: URL: https://github.com/apache/iceberg/pull/16943
Closes #16942 ## Problem `createOrReplace` and `replace` build their replacement metadata from the table state captured when the transaction starts, and `TableMetadata.buildReplacement` keeps the existing snapshot history. When two such transactions run concurrently, the one that loses the optimistic-lock race retries in `BaseTransaction.commitReplaceTransaction`, but the retry advanced `base` to the refreshed metadata while re-committing the stale replacement built from the original base. As a result, any snapshot the concurrent writer committed in between was silently dropped from history. This contradicts the documented behavior that replacing a table keeps its history (`docs/docs/spark-ddl.md`) and is inconsistent with sequential replace, which preserves all snapshots. It affects full-metadata-rewrite catalogs (Hadoop, Hive, Glue, JDBC, Nessie, in-memory); REST already merges these changes server-side. See #16942 for the minimal reproduction. ## Fix On a replace commit retry, when a concurrent change is detected, the replacement metadata is rebuilt on top of the refreshed table and the transaction's pending updates are re-applied. The concurrent writer's snapshots stay in history while the replacement still becomes the current state, mirroring how the simple-transaction path re-applies its updates after a refresh. The rebuild is skipped when the concurrent change altered the schema or partition spec, since re-running `buildReplacement` would reassign field ids and break data this transaction already wrote; in that case the existing last-writer-wins behavior is retained. Catalogs that merge changes server-side (REST) pass no rebuild function and are unaffected. Because the replacement is now rebuilt on the refreshed base, a concurrent writer's property updates committed during a replace transaction are also preserved on retry, where the metadata-rewrite catalogs (Hadoop, Hive) previously clobbered them. `buildReplacement` overlays the replace's properties on top of the base it is built from, so a concurrently-set property that is already on the refreshed base is carried forward. This brings those catalogs in line with REST, which applies the replace as a delta and never removes a concurrently-set property server-side. ## Tests Added `TestReplaceTransaction.testReplaceTransactionConcurrentCommitRetainsHistory`, which forces a concurrent commit during a replace and asserts the concurrent writer's snapshot stays in history while the replacement wins the current state. It fails without the fix and passes with it across all format versions. The existing concurrent-replace coverage in `CatalogTests` (including the schema and partition-spec variants) continues to pass for in-memory, JDBC, and REST catalogs. `TestHiveCreateReplaceTable.testReplaceTableTxnTableModifiedConcurrently` was updated to assert that the concurrent property update is now preserved, reflecting the REST-consistent behavior described above. --- **AI Disclosure** - Model: Claude Opus 4.8 - Platform/Tool: Claude Code - Human Oversight: fully reviewed - Prompt Summary: Fix createOrReplace and replace transactions dropping concurrently committed snapshots by rebuilding the replacement metadata on the refreshed base. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
