pecsorabs commented on issue #1893: URL: https://github.com/apache/iceberg-rust/issues/1893#issuecomment-6628150595
I'd like to pick up task 1, `update_table` for the HMS catalog, unless someone is already on it. We have a working implementation outside this repo. [faucet-stream](https://github.com/faucet-hq/faucet-stream) (Apache-2.0/MIT) wraps `HmsCatalog` to commit through the metastore, and its integration tests run against an `apache/hive:3.1.3` standalone metastore. We'd rather drop that wrapper and use `HmsCatalog` directly, so I'd like to bring the implementation here. It follows Java's `HiveTableOperations` with locking on: 1. Take an `EXCLUSIVE` table lock (`lock`, then poll `check_lock` with backoff while it's `WAITING`, up to a timeout). 2. Load the table, check the commit's requirements, and apply its updates. The new metadata goes to `metadata/<version+1>-<uuid>.metadata.json`. 3. Write that file through the table's `FileIO`. 4. Re-read the metastore entry and refuse unless `metadata_location` is still the one the commit was based on. 5. `alter_table` to the new `metadata_location`, set `previous_metadata_location`, and pass `DO_NOT_UPDATE_STATS`. 6. `unlock`, also on every error path. A commit that loses the race (the table moved, the lock was refused or never granted, or a requirement no longer holds) returns `CatalogCommitConflicts`, so `Transaction::commit` retries under the `commit.retry.*` properties. A failure after `alter_table` was sent but not answered is ambiguous and is not marked retryable, since a retry could append the same data files twice. Before returning a conflict, the commit deletes the metadata file it staged. Without tests it's roughly 550 lines, so to stay within the PR size guideline I'd split it: - **PR 1:** `update_table` with the lock flow above, a lock-timeout catalog property (Java uses `iceberg.hive.lock-timeout-ms`), unit tests for the lock-state handling and the optimistic check, and an integration test against the existing Hive test setup (concurrent commits from two catalogs, one losing and retrying). - **PR 2:** a lockless path for Hive 4, using `alter_table` with an expected-parameter environment context as an atomic compare-and-set on `metadata_location` (Java's `iceberg.engine.hive.lock-enabled=false`). It builds on #3187. Two questions before I start: - Should this wait for #3104 (HMS configuration from properties), or go in first? - Any preference on the lock-timeout property name? I'd match Java's `iceberg.hive.lock-timeout-ms` unless there's a reason not to. -- 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]
