wgzhao commented on PR #13111: URL: https://github.com/apache/seatunnel/pull/13111#issuecomment-6624813675
Thanks @DanielLeens for the re-review and the approval, and for confirming the 2PC E2E result. I agree with both observations. Unless you would prefer them in this PR, I would like to keep the current head `5b08306819` unchanged and address them in one follow-up PR: - **Upgrade-window orphan:** the window only exists on the first restore from a pre-upgrade checkpoint. I will add the extra `abortPreCommit(labelPrefix, ...)` for the current prefix and make the restore test assert both calls. I want this in before the upstream fix is deployed onto an existing job, so it will be the first item in the follow-up. - **CRC32 collision:** for typical jobs the probability is negligible; for very large many-to-one 2PC jobs I will either move to 12-hex SHA-256 or document the limit in the FAQ. If we change the hash, it creates another prefix transition, so it should land together with the dual-abort fix. The main reason for not putting them here is CI cost and review state: a new commit dismisses your approval and re-runs the full Doris IT (about 75 minutes), while both items are explicitly non-blocking and this head already fixes #13108 and has the 2PC regression coverage. Keeping this PR to one problem also matches the project's PR scope rule. If you would rather have either item in this PR before merge, please say so and I will fold it in here; otherwise I will open the single follow-up after this one merges. -- 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]
