SEZ9 commented on issue #12441: URL: https://github.com/apache/seatunnel/issues/12441#issuecomment-5862165474
@Rangsh thanks, this is what I asked for. The part 1 table covers the three things I wanted recorded: the test (`CheckpointCoordinatorTest#testTryTriggerNotBlockedBySavepointDrain`), the red base (`c3f06f9a82c119e427df0fb1f440bae583148f62` on `dev`), and the failure mode. Owning the drain lifetime through `pendingCounter` instead of wall-clock sleeps is the right way to keep it deterministic. Two small asks on the red run before I review https://github.com/apache/seatunnel/pull/12493: 1. Please make sure the same table (test, base revision, failure) is in the PR description itself, not only here, so the red run travels with the change. 2. For the "Red" row, please paste the actual observed failure from running the test against `c3f06f9a82c119e427df0fb1f440bae583148f62` (the assertion or timeout output), rather than the summary "blocked for the full drain". Along with that, state the bound the green side asserts on for `tryTriggerPendingCheckpoint` returning, so it is clear it is well below the drain time. Part 2 ordering is as agreed: it stays pending on #12165, I will write the cross-pipeline regression once that is in, and this issue remains open until part 2 has gone red then green. Keeping #12442's trigger-failure policy out of this change is also correct. I will go through the PR once the description is updated. <!-- streview-comment:1362 --> -- 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]
