rkhachatryan commented on pull request #13040: URL: https://github.com/apache/flink/pull/13040#issuecomment-674026613
Thanks for the update @echauchot I see there are some git conflicts now, can you rebase your branch to the latest master? Besides, there are some checkstyle violations (CI is still failing). > 3. I think we can re-structure the code so that ZKStore doesn't depend on CheckpointCleaner; for this, include a call to CheckpointCleaner into CompletedCheckpoint.discardCallback > Ah, you mean calling increment in the executor thread not in the other thread (See #13040 (comment))? No, I mean there are 1. cyclic dependency (coordinator <-> cleaner through a callback) which makes us use non-final volatile 1. dependency on "different levels" (ZKStore and cleaner) which makes us pass the cleaner while building execution graph. Let me illustrate both the problem and a possible solution with a diagram:  > 4. it is just a delegate > Isn't the UTest you want the test that checkpoint request is triggered after deletion completes ? I don't it's "just a delegate" because it contains this counter and, more importantly a callback. I think these should be covered by tests. And as the whole feature involves multiple components, I think we need an integration test. 5. `PendingCheckpoint.executor` is not used anymore so it can be deleted Thanks for letting know about your timing. Have a good time :) ---------------------------------------------------------------- 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]
