SEZ9 commented on issue #12441:
URL: https://github.com/apache/seatunnel/issues/12441#issuecomment-5964403529

   @Rangsh — thanks for running both verification points on #12511 and writing 
them up this clearly.
   
   On point 1, the carry-over is accepted. Re-applying 
`testTryTriggerNotBlockedBySavepointDrain` onto the #12511 tip and showing it 
red on `c3f06f9a82c119e427df0fb1f440bae583148f62` with the 
`tryTriggerPendingCheckpoint must return while savepoint drain is still held` 
assertion, then green on `91001c42caeb25ae1a44cc3b7eeeab1c57201cfb`, is exactly 
the proof I was after. It is also fine that the test on #12511 itself is named 
`testTriggerDuringSavepointDrainReturnsAndRearmsWithoutCreatingACheckpoint`; 
the behaviour under test is the same.
   
   On point 2, the three-row table answers the restore-race question. The 
source timings you pulled (4 s `prepareCommit` failure, 5 s `checkpoint_sleep`, 
3 s `JOB_RETRY_INTERVAL_SECONDS` default) line up with the scenario, and seeing 
`testSavePointFailureDuringPipelineRestoreWaitEndsTheJob` red on the drain-only 
commit `146a1b5c5`, red on `dev` at `4c874e2a4061aea9d5db65e74edeb211b498fe27`, 
and green on the full #12511 tip isolates the `isNeedRestore()` re-check as the 
piece that fixes it. Nothing more is needed from you on either of these two 
points.
   
   What's still open is unchanged from before: #12511's Build is failing, so 
the fix is not in `dev` and I'm keeping this issue open. Remaining asks:
   
   1. Get the #12511 Build green, or post concrete evidence that the failure is 
unrelated to the savepoint changes.
   2. Keep both red → green regressions (the drain test and 
`testSavePointFailureDuringPipelineRestoreWaitEndsTheJob`) intact on the branch 
as it is updated so they can be re-checked at review time.
   
   Closing #12493 in favor of #12511 stays the right call; please don't reopen 
a parallel drain PR. If you have cycles to help land the Build fix on #12511, 
that's the most useful place to put them.
   
   <!-- streview-comment:1476 -->


-- 
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]

Reply via email to