dybyte commented on PR #11494: URL: https://github.com/apache/seatunnel/pull/11494#issuecomment-5038686552
> Thanks for the update. I re-reviewed the complete current-head diff and the affected runtime path from scratch. The direction is useful, but the following items still block merge: > > 1. **Blocker 1: Equal-timestamp WAL mutations still have no stable total order** (`seatunnel-engine/seatunnel-engine-storage/imap-storage-plugins/imap-storage-file/src/main/java/org/apache/seatunnel/engine/imap/storage/file/common/LatestMutationAccumulator.java:47-55`) — The latest commit switched to `mutation.compareTo(current) < 0`, but `IMapFileData.compareTo()` returns `-1` even for equal objects. Both directions compare as smaller and `x.compareTo(x) != 0`, so recovery now chooses the last filesystem traversal result rather than a durable latest mutation. Please persist a cross-writer/region comparable epoch plus sequence, use a deterministic per-key order, and test equal timestamps across files in opposite traversal orders. > > The current Build is `PENDING`. Please rerun the required checks after the source changes. > > Thanks again for working through this — happy to re-review the next head. > > Queue note: this head is `diverged` against `dev` (ahead_by=3, behind_by=4). I am recording that fact separately; it is not by itself a request to sync unless the CI guidance above explicitly says so. The equal-timestamp ordering issue is pre-existing in the current WAL recovery path and is caused by the current IMapFileData.compareTo() implementation. The comparator contract and deterministic ordering for equal-timestamp mutations are being fixed separately in #11465. This PR intentionally reuses IMapFileData.compareTo() so that, once #11465 is merged and this branch is updated, the streaming recovery path will preserve the same ordering semantics as the full-sort recovery path. Persisting a cross-writer/region epoch and sequence would require a broader WAL format and writer protocol change. That limitation already exists in the current implementation and is not introduced by this PR. If strict causal ordering across writers is required, I believe it should be tracked separately from this memory-usage optimization. -- 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]
