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

   @Vivek1106-04 thanks, that answers bug 2 for me.
   
   The classification you describe is the behavior I was after: a draining 
savepoint that gets cut short by `cleanPendingCheckpoint` has not created its 
checkpoint yet, so collapsing every close reason 
(`CHECKPOINT_COORDINATOR_RESET`, `PIPELINE_END`, 
`CHECKPOINT_COORDINATOR_COMPLETED`, and the `handleCoordinatorError` path) to 
`CHECKPOINT_COORDINATOR_SHUTDOWN` keeps it inside the 
`JobMaster.isSavepointStartPreconditionException` contract and matches what 
`dev` returned when its drain loop saw `shutdown`. Keeping the "checkpoint 
created, then aborted" case as a real failure with the abort reason propagated 
unchanged through the `PendingCheckpoint` is also the right split. 
Parameterizing 
`testCoordinatorResetDuringSavepointDrainFailsTheRequestAndResumesTriggering` 
over all four reasons as of `a691fb195` is exactly the pin I wanted on that, so 
nothing further is needed on the classification itself.
   
   That leaves the one open item from the earlier review: the Build failure on 
#12511. You mention the branch is rebased onto current `dev` and CI is running 
on the new head; once it finishes, please post the outcome here. If anything is 
still red, either fix it or document concrete evidence that it is unrelated to 
this change. Please also confirm that both red-to-green regressions (the 
drain-gate test Rangsh ported and checked red/green in 
https://github.com/apache/seatunnel/pull/12511#issuecomment-5977952591, and 
`testSavePointFailureDuringPipelineRestoreWaitEndsTheJob`) still pass on the 
rebased head.
   
   Until that is in, this issue stays open and the fix should not be treated as 
present in `dev`.
   
   <!-- streview-comment:1519 -->


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