pnowojski commented on a change in pull request #9853:
[FLINK-13904][checkpointing] Avoid competition of checkpoint triggering
URL: https://github.com/apache/flink/pull/9853#discussion_r335425982
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -535,149 +528,142 @@ public boolean isShutdown() {
// we will actually trigger this checkpoint!
- // we lock with a special lock to make sure that trigger
requests do not overtake each other.
- // this is not done with the coordinator-wide lock, because the
'checkpointIdCounter'
- // may issue blocking operations. Using a different lock than
the coordinator-wide lock,
- // we avoid blocking the processing of 'acknowledge/decline'
messages during that time.
- synchronized (triggerLock) {
Review comment:
Can we get rid of the `triggerLock` here? With the code as it is,
`CheckpointCoordinator#triggerCheckpoint(long, CheckpointProperties,
java.lang.String, boolean, boolean)` method can be called from the `timer`
thread (it's single threaded now) and also from the main thread via
`triggerSavepointInternal`, right?
Shouldn't this change happen after we get rid of the timer thread altogether?
Also this commit deserves a better/more detailed explanation in the commit
message. Could you write/explain there why can we drop the lock?
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
