SEZ9 commented on PR #11819: URL: https://github.com/apache/seatunnel/pull/11819#issuecomment-5658325246
Quick housekeeping note: the two review submissions pullrequestreview-5193492675 and pullrequestreview-5193493027 are byte-identical duplicates caused by a tooling retry. Please treat pullrequestreview-5193492675 as the authoritative one and ignore the second — there is no additional feedback in it. To keep things clear, the open items from that review remain as previously listed (PR11819-F1 through PR11819-F8). Concretely, what I'd still like to see or hear back on: - **F1 / F2 / F6 (MultipleTableFileSourceReader):** make a rotation or deletion detected during/after a read an isolated skip for that split rather than a whole-reader failure, and avoid emitting rows to the collector before the identity check confirms the file is still the one we opened. Please also consider whether other transient I/O failures on a single tailed file should be tolerated the same way the pre-read `NoSuchFileException` is. - **F5 (MultipleTableFileSourceReader):** the reader-side identity check is TOCTOU and inode reuse makes file-key equality insufficient on its own — please describe how you'd like to handle that, or document the limitation explicitly if you consider it acceptable. - **F7 (MultipleTableFileSourceReader):** skipped tail splits should not report a stale content fingerprint from a prior read in `FileSplitFinishedEvent`. - **F4 (LocalFileIdentity):** `fileKey.toString()` (dev+inode) is not stable across reboots/remounts; please explain the intended behavior on checkpoint restore, or switch to / combine with a more stable identity. - **F3 (FileSplitFinishedEvent / FileSourceSplit / FileSourceState):** please confirm (ideally with a test) that state restore from a pre-upgrade checkpoint still works with the newly added serialized fields. - **F8 (ReadStrategy):** add Javadoc for `processedBytes` and `getLastReadBytes()` spelling out the `-1` vs `0` contract. Happy to re-review once these are addressed or if you'd like to discuss any of them further. <!-- streview-comment:1031 --> -- 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]
