goutamadwant commented on PR #12449: URL: https://github.com/apache/seatunnel/pull/12449#issuecomment-5841261576
I ran `CheckpointCoordinatorFailoverIT#testStreamJobFailsAfterCheckpointTriggerDispatchFailure` in a loop locally. I compared `dev`, this PR, #12288, and the matching part of #12454. Setup: macOS, 10 cores, one fresh JVM per run. JDK 11 runs used `-XX:ActiveProcessorCount=2`. Base was `dev` at `deb16a3c3`. Each head was the PR diff applied to that commit. | variant | JDK 11, loaded host | JDK 11 + 6 CPU burners | JDK 8 | |---|---|---|---| | `dev` | 3 / 40 | 7 / 30 | 0 / 8 | | this PR | 0 / 40 | 0 / 30 | 0 / 8 | | #12288 | 2 / 40 | 5 / 30 | 1 / 8 | | #12454 | 0 / 40 | 0 / 30 | 0 / 8 | All failures on `dev` and #12288 are the CI one: `Checkpoint notify complete failed` instead of `CHECKPOINT_INSIDE_ERROR`. That means the entry was removed while a completion notification was still in flight. Reading the code, the fix holds. `completePendingCheckpoint` decrements `pendingCounter` only after `notifyCompleted` returns. `tryTriggerPendingCheckpoint` checks and increments it under the same `lock`. So "counter is 0 under the lock" is a real between-checkpoints point. The two new assertions also make the test stricter, not looser. This PR and #12454 inject the fault at the same point. #12454 does it through a `@VisibleForTesting` method on `CheckpointCoordinator`; this PR uses reflection in the test only. Both were 0 failures here. Landing one and dropping the other seems best; this one has the advantage of not touching engine code. -- 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]
