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

   Thanks for the detailed analysis. The race looks plausible: 
`restoreCoordinator` swaps `checkpointCoordinatorFuture` and 
`errorByPhysicalVertex` while deferred handlers on the coordinator executor or 
checkpoint timer thread may still be in flight, and `handleCoordinatorError` 
only checks whatever future is current when it runs.
   
   The planned fix sounds reasonable: have each deferred handler capture the 
current future when its work starts and call an owner-checked 
`handleCoordinatorError(owner, ...)` under the same `generationLock` that 
#12165 introduces for the timeout path, so a stale error is logged and dropped 
rather than failing the restored generation. Guarding the 
`errorByPhysicalVertex` write with the same check makes sense too.
   
   The held-executor regression tests you describe (stale error released after 
a restore leaves the restored generation running; the same error without a 
restore still fails it) are a good way to lock this in. Happy to review the PR 
once #12165 merges.
   
   <!-- streview-comment:2110 -->


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