SEZ9 commented on PR #12288: URL: https://github.com/apache/seatunnel/pull/12288#issuecomment-5842623650
Thanks @goutamadwant for running the loop — that's exactly the evidence point 5 was missing. The numbers match the window (a) analysis: with the diff applied on top of `deb16a3c3`, this PR only moves the failure rate from 3/40 to 2/40 on JDK 11 (5/30 vs 7/30 under CPU pressure), and every remaining failure is the same `Checkpoint notify complete failed` message, now at line 857. That is what you'd expect when `latestCompletedCheckpoint` is written before checkpoint 1's own `notifyCompleted` runs — polling `getLatestCompletedCheckpointId()` and then removing the `ownedSlotProfilesIMap` entry can still race the in-flight notification, so this PR narrows the window but doesn't close it. Waiting for `pendingCounter == 0` and doing the removal under the coordinator lock removes the race entirely, and 0/40, 0/30, 0/8 across your three configurations is consistent with that. Not needing a new production accessor is a further plus. Given that, I agree with consolidating on the alternative PR you tested. I have no further changes planned on top of `25693bef`, so I'll close this one as superseded. Two small asks before I do: 1. @goutamadwant, could you post the exact loop setup (JVM flags, `-XX:ActiveProcessorCount=2`, burner setup, base `deb16a3c3`) as a comment on the surviving PR? Having the reproduction attached there makes it much easier to re-verify if the test ever flakes again. 2. I'll carry the `45a8993b` point from the earlier discussion over to that PR so nothing from this thread gets lost. Thanks again for the thorough comparison. <!-- streview-comment:1325 --> -- 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]
