StephanEwen commented on a change in pull request #11274:
[FLINK-16177][checkpointing] Integrate OperatorCoordinator checkpoint
triggering and committing
URL: https://github.com/apache/flink/pull/11274#discussion_r387531629
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -291,10 +292,8 @@ public CheckpointException getFailureCause() {
public CompletedCheckpoint finalizeCheckpoint() throws IOException {
synchronized (lock) {
- checkState(areMasterStatesFullyAcknowledged(),
- "Pending checkpoint has not been fully
acknowledged by master states yet.");
- checkState(areTasksFullyAcknowledged(),
- "Pending checkpoint has not been fully
acknowledged by tasks yet.");
+ checkState(!isDiscarded(), "checkpoint is discarded");
+ checkState(isFullyAcknowledged(), "Pending checkpoint
has not been fully acknowledged yet");
Review comment:
True, but this is mainly a "sanity check" (guarding against accidental bugs)
that should never actually trigger, so the error message detail is not so
critical.
I found that previous changes (master hooks) were forgotten to add there, so
this "check for all acks" seemed more safe/maintainable to me, compared to
updating this whenever something would change.
----------------------------------------------------------------
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
