SEZ9 commented on PR #11819: URL: https://github.com/apache/seatunnel/pull/11819#issuecomment-5628821934
Quick follow-up on the open review points, since there is no new commit beyond `74340c7626` yet. The automated trace of the read path lines up with PR11819-F5 (and the related F1/F2): `MultipleTableFileSourceReader.pollNext` (`MultipleTableFileSourceReader.java:99-116`) only compares `LocalFileIdentity` before and after the read, and that identity (`fileKey`, dev+inode) survives a truncate-and-rewrite. `endContentAnchor` is populated on `FileSourceSplit` and used in `equals()`/`hashCode()`, but it is never compared against the file's current state in the read path. So a rewrite that lands between split assignment and split read passes both identity checks while the bytes underneath have changed, and because rows are emitted before the post-read check, replaced content can already have reached the sink. I'm treating my earlier approval as superseded until this is addressed. Concrete asks before I re-review: 1. **F5 / F2** – Validate the split's content anchor against the file before emitting rows (or buffer and discard on mismatch), instead of relying on `fileKey` equality alone. 2. **F1 / F6** – On an identity mismatch or any other transient I/O failure for a single tailed file, skip/re-enqueue that split and log it, rather than throwing an `IOException` that fails the whole reader task. 3. **F7** – When a tail split is skipped, don't report the fingerprint from a previous read in `FileSplitFinishedEvent`; use an explicit "not read" marker. 4. **F3** – Confirm, ideally with a test, that restoring from a pre-upgrade checkpoint works with the new serialized fields on `FileSplitFinishedEvent`, `FileSourceSplit` and `FileSourceState`. 5. **F4** – Document or mitigate the inode-reuse / remount instability of `LocalFileIdentity` on restore (e.g. fall back to the content anchor when the identity no longer matches). 6. **F8** – Add Javadoc on `ReadStrategy` for `processedBytes` / `getLastReadBytes()` spelling out the `-1` vs `0` contract. Happy to take another pass as soon as a commit lands. <!-- streview-comment:958 --> -- 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]
