yanghua commented on a change in pull request #8322: [FLINK-12364] Introduce a
CheckpointFailureManager to centralized manage checkpoint failure
URL: https://github.com/apache/flink/pull/8322#discussion_r283758545
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -435,6 +439,12 @@ public boolean triggerCheckpoint(long timestamp, boolean
isPeriodic) {
triggerCheckpoint(timestamp, checkpointProperties,
null, isPeriodic, false);
return true;
} catch (CheckpointException e) {
+ try {
+ long latestGeneratedCheckpointId =
getCheckpointIdCounter().getAndIncrement();
Review comment:
> If you read my first comment, it was not about that you get the value from
that counter, but I wondered why you do `getAndIncrement` instead of just `get`.
Actually, I also want to use `get` API, but `CheckpointIDCounter ` does not
provide `get` API.
> Rest of my comments still holds though, why not remove the
throwing/catching of an exception that we can also directly report to the
failure manager? I also think that makes other counts accurate because it
obtaining the id can then always happen under the lock.
OK, agree.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
