DanielLeens commented on PR #12288: URL: https://github.com/apache/seatunnel/pull/12288#issuecomment-5846306650
Thanks both — this settles it. @goutamadwant, that loop is exactly the evidence point 5 was missing, and it's decisive: with this PR's diff applied on top of `deb16a3c3`, the failure rate only moves from 3/40 to 2/40 on JDK 11 (5/30 vs 7/30 under CPU pressure), and every remaining failure is still `Checkpoint notify complete failed`. That's precisely window (a) from my last comment — `latestCompletedCheckpoint` is written before checkpoint 1's own `notifyCompleted` call finishes, so polling `getLatestCompletedCheckpointId()` and then removing the `ownedSlotProfilesIMap` entry can still race that in-flight notification. This PR narrows that window with a new test-only accessor; #12449's approach — wait for `pendingCounter == 0` and do the removal under the coordinator lock — closes it outright and needs no production accessor at all. With 0/40, 0/30, 0/8 across your three configurations, that's clearly the better fix. @SEZ9, agreed — I have no further changes planned on top of `25693bef` either. I'll close this PR as superseded by #12449 and carry the `45a8993b` point from goutamadwant's original review over to that thread so it isn't lost. @goutamadwant, if you could post your exact loop setup (JVM flags, `-XX:ActiveProcessorCount=2`, burner configuration, base `deb16a3c3`) as a comment on #12449, that will make it much easier to re-verify if the test ever flakes again. Thanks to both of you for chasing this all the way down to a real root cause instead of settling for the first plausible-looking fix. -- 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]
