gaoyunhaii commented on pull request #16905: URL: https://github.com/apache/flink/pull/16905#issuecomment-904582245
Hi Piotr @pnowojski , very thanks for reviewing this PR! As a whole I think you are right that we should go with the `syncSavepointWithDrain` method, and sorry Initially I did not noticed this part of logic. In detail, I think currently we should have two issues: 1. We might set the `syncSavepointWithDrain` after stopping the operators, as already being pointed out. 2. We might need to keep the calling to `mainOperator.stop()` inside the mailbox thread to avoid concurrency access to the operators. I'll update the PR with this method~ And just to make it clear, I think with the current method since `triggerCheckpiontAsync` is added in a future after `operator.finish()` in the mailbox thread, thus we could ensures `triggerCheckpiontAsync` get called immediately after called `operator.finish()`, and the mail must be processed at least in the `mailboxProcessor.runMailboxLoop();` in line 849 before closed the mailbox. But since we have already have the logic related to `syncSavepointWithDrain` , I also agree it is more reasonable to use this method. -- 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]
