Myasuka commented on pull request #8693: URL: https://github.com/apache/flink/pull/8693#issuecomment-621674425
@rkhachatryan Thanks for your review! I'll rebase master code on top of https://github.com/apache/flink/pull/11721 recently. 1. > does it make sense to split the commit (or even PR) into several pieces addressing JM and TM separately? I also previous single commit is a bit large, I'll split into several smaller ones. 2. > can you add a component name to the commit message? Sure, no problem. It was my fault to missing component information. 3. > AsyncCheckpointRunnable.cancelled seems not to be used. And why not to add a new AsyncCheckpointState instead (or reuse DISCARDED)? Previous `AsyncCheckpointRunnable.cancelled` is used for testing to verify whether this async runnable is cancelled. The reason why I not add a new `AsyncCheckpointState` is due to I reuse `AsyncCheckpointRunnable#close` to cancel the running thread which would set the state as `DISCARDED`. I think this is an open question to discuss to search for a better solution. 4. > I don't see how AsyncCheckpointRunnable is canceled or its thread is stopped; which is the original goal of this PR I believe As I said above, I reuse `AsyncCheckpointRunnable#close` to cancel the running async checkpoint task. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
