GGraziadei commented on PR #8950: URL: https://github.com/apache/storm/pull/8950#issuecomment-5302155601
Every point is answered inline; pushed as fb42419. Summary of what changed, since one of them is a behaviour change rather than a fix. **Recovery no longer replays.** Your argument on `IcebergCommitter:146` holds: an entry only survives to startup if the batch was never acked, so the source replays it anyway and appending the entry's files can only add a second copy. `recover()` now abandons every pending entry and lets its files orphan. That also removes the unbounded/unquarantined replay from `prepare()` and the dependence on a snapshot `expire_snapshots` may have removed — both by construction rather than by patch. **Fixed** - `IcebergWriter.abort()` catches `RuntimeException` as well as `IOException`, so a `FileIO` delete failure can no longer escape a caller that is in the middle of failing a batch, and `close()` reaches the catalog shutdown. This was the most concrete one and it is covered by a test that fails without the fix. - `complete()` releases the writer only on success, so the abort in the commit-failure path is no longer a no-op. - WAL deletes after a landed append no longer propagate, so a visible commit is not replayed because its entry could not be removed. - `IcebergBolt` declares its own tick, so `withTickIntervalSecs` is no longer silently dropped. - `withWalNamespace(String)` separates the commit WAL of deployments sharing one table. - Docs: the exactly-once claim is gone from the did-not-land branch and from the caveats; the unknown-state path is described as it actually behaves. The "upgrading from an earlier layout" sections are removed from both files, and `docs/index.md` now links the page. **Not changed, with reasoning inline** - The poison-tuple escape hatch — an API decision I would rather agree first. - `pendingFiles` volatility: the field is mutated in place and never reassigned, so volatile would publish nothing the gauge reads. - The WAL location sort: now decorative, since recovery no longer replays in order. 68 tests, checkstyle and PMD clean on JDK 25 (`mvn verify -pl external/storm-iceberg,examples/storm-iceberg-examples`). Six of them are new: the writer's abort paths, the WAL namespace, the bolt's tick declaration, and the WAL-delete-after-landed-append case. -- 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]
