SEZ9 commented on PR #12623: URL: https://github.com/apache/seatunnel/pull/12623#issuecomment-6552162439
@Vivek1106-04 thanks for the detailed follow-up and for posting the fork Build result for `caa5f098f` (run 40727927696, `success`, head `caa5f098fc0a5c6dd887ba8632dc43d463eb4580`). **F1 (restore thread after the sleep).** I follow the reasoning: the post-sleep `isNeedRestore()` re-check lives in #12511 at `a691fb195`, and this PR is meant to land after it. That said, this PR's diff on its own still leaves the base without the guard, so I'd rather not mark F1 resolved here. Could you either add a short note in the PR description that this change depends on #12511 landing first, or confirm you're fine with me re-checking F1 once #12511 is merged? **F2 (pipeline-level proof).** Same situation: the #12622 whole-job scenario is covered by tests in #12511, not in this PR. The vertex-level `StateTransitionCleanupTest` cases here make sense for the post-recheck window you describe. For the claim that they fail when the `PhysicalVertex` change is reverted, could you paste the failing output from a run with the change reverted? That would let me close F2 on this PR without depending on the companion. Nothing else outstanding from my side beyond these two points. <!-- streview-comment:2176 --> -- 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]
