kotwal-itpro commented on PR #8819: URL: https://github.com/apache/hop/pull/8819#issuecomment-6086013246
Thanks @mattcasters and @bamaer, you were both right on all of these. Fixed in c89a7c1407 (rebased onto main): **Two writers committing the same version.** `PathTableOperations` no longer uses VFS `moveTo`. On a local file system the new version is published with `Files.createLink(target, temp)`, which fails with `FileAlreadyExistsException` if the name is taken, atomically, also across processes; where hard links aren't supported it falls back to `Files.move` without `REPLACE_EXISTING`. The temp file is deleted in a `finally`. Commits within one JVM are also serialized per table location with a lock, like the `LockManager` in `HadoopTableOperations`. Other file systems keep check-and-move, so the Javadoc now says path tables there need one writer at a time (object stores can't do better, as you said). `PathTableOperationsTest.concurrentWritersInOneJvmLoseNoSnapshot` runs 8 writers with 5 appends each against one table and checks that every commit that reported success is in the table. On the old code it failed on the third run with 37 successful commits and 36 snapshots, which is exactly the lost snapshot you described; with the fix it passes. **Stale version hint.** `IcebergTables.findCurrentMetadataFile` now treats the hint as a starting point and walks `v<N+1>`, `v<N+2>`, ... (and `.gz.metadata.json`) to the newest version, like `HadoopTableOperations`; an unreadable hint falls back to the folder listing. `staleHintIsFollowedForward` leaves the hint at 1 with `v3` on disk, checks that readers see v3, and that the next commit writes v4 and moves the hint to 4. I replaced the old `versionHintWins` test, which asserted the behavior you flagged. **Deleting files after a commit took effect.** Once the metadata file is in place, a failed hint write is logged and the table is read back lazily, so the commit can't fail after that point. `CommitCoordinator` now records whether the snapshot was published; `LakeTableOutput` only calls `abort()` when it wasn't, and `abort()` itself refuses once the snapshot is published or the outcome is unknown. A `CommitStateUnknownException` keeps the files and fails the run with a message to check the table history before rerunning. `CommitCoordinatorTest` covers the unknown, published-then-refresh-failed and definite-failure cases. **Unknown save mode.** Only `Append`, `Overwrite`, `ErrorIfExists` and `Ignore` are accepted; anything else fails before writing, for an existing table and a new one (tests for both). Tests: lakehouse 85 (was 76), Spark 179, RAT clean. -- 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]
