Rangsh commented on PR #12511: URL: https://github.com/apache/seatunnel/pull/12511#issuecomment-5944739904
## Independent verification (SEZ9 ask on #12441) Re-ran the two checks you asked for against this PR. Summary below. ### 1. Part-1 carry-over: `CheckpointCoordinatorTest#testTryTriggerNotBlockedBySavepointDrain` |#12511 renamed the equivalent coverage to `testTriggerDuringSavepointDrainReturnsAndRearmsWithoutCreatingACheckpoint`; I also re-applied the original `#12493` method name onto this tip so the exact test SEZ9 named can be re-run.| | | | |---|---| | **Test** | `CheckpointCoordinatorTest#testTryTriggerNotBlockedBySavepointDrain` | | **Red base** | `c3f06f9a82c119e427df0fb1f440bae583148f62` (unfixed coordinator + the `#12493` test) | | **Red failure** | `AssertionFailedError: tryTriggerPendingCheckpoint must return while savepoint drain is still held (pendingCounter>0); blocked for the full drain indicates the lock is held across sleep-poll ==> expected: <true> but was: <false>` | | **Green tip** | `#12511` @ `91001c42caeb25ae1a44cc3b7eeeab1c57201cfb` | | **Green** | `Tests run: 1, Failures: 0, Errors: 0` | Part-1 proof carries over from `#12493` to `#12511`. ### 2. SubPlan restore race (4 s / 5 s vs 3 s) **Timing check (source):** - failing sink: `InMemorySinkWriter.prepareCommit` → `Thread.sleep(4000L)` then throw (`throw_exception`) - slow sink: `Thread.sleep(5000L)` (`checkpoint_sleep`) - restore wait: `EnvCommonOptions.JOB_RETRY_INTERVAL_SECONDS` default **3** That matches the conf/test contract: stop from the 4 s failure must land inside the failed pipeline’s 3 s restore wait, while the other pipeline is still finishing its 5 s savepoint work. | Scenario | Revision / tree | `testSavePointFailureDuringPipelineRestoreWaitEndsTheJob` | Observed | |---|---|---|---| | unlocked drain **without** `isNeedRestore()` re-check | `#12511` drain commit on `146a1b5c5` (no SubPlan re-check) | **RED** | `expected: <FAILED> but was: <DOING_SAVEPOINT>` (60s timeout). Timeline: savepoint → pipeline-1 fails ~**+4 s** → restart after ~**+3 s** restore wait → job stuck in `DOING_SAVEPOINT` | | current `dev` (test cherry-picked, no SubPlan fix) | `4c874e2a4061aea9d5db65e74edeb211b498fe27` | **RED** | same: `expected: <FAILED> but was: <DOING_SAVEPOINT>`; fail ~+4 s, restore restart ~+3 s | | `#12511` full (unlocked drain **+** re-check) | `91001c42caeb25ae1a44cc3b7eeeab1c57201cfb` | **GREEN** | after the 3 s wait: `no longer needs restore, ending it as FAILED instead of restarting it` → job leaves `DOING_SAVEPOINT` → `FAILED` | **`SavePointTest#testSavePointButJobGoingToFail`:** - **GREEN** on `#12511` full (`91001c42c`) together with the restore-wait test (`Tests run: 2, Failures: 0`). - On unlocked drain **without** the re-check, two isolated runs of this single test still passed. The race is reliably exposed by `testSavePointFailureDuringPipelineRestoreWaitEndsTheJob` (the 4 s / 5 s / 3 s case), which is the one that stayed red without the re-check and went green with it. Happy to dig further if you want more `testSavePointButJobGoingToFail` retries under a full `SavePointTest` class run. -- 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]
