pnowojski commented on a change in pull request #17278:
URL: https://github.com/apache/flink/pull/17278#discussion_r712972528
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -323,10 +323,6 @@ public CheckpointCoordinator(
try {
this.checkpointStorageView =
checkpointStorage.createCheckpointStorage(job);
-
- if (isPeriodicCheckpointingConfigured()) {
- checkpointStorageView.initializeBaseLocationsForCheckpoint();
- }
Review comment:
Secondly, now I guess we will be failing a bit later and first
checkpoint can theoretically take longer as a result of this change?
Would it be complicated to initialise base
`initializeBaseLocationsForCheckpoint()` if
`isPeriodicCheckpointingConfigured()`, but if
`isPeriodicCheckpointingConfigured()` is false, initialise it also in the
`FsCheckpointStorageAccess#initializeLocationForCheckpoint()` the way you are
doing it? It would definitely complicate the system by having two ways of
initialisation, so I'm not yet convinced which option is better.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
##########
@@ -323,10 +323,6 @@ public CheckpointCoordinator(
try {
this.checkpointStorageView =
checkpointStorage.createCheckpointStorage(job);
-
- if (isPeriodicCheckpointingConfigured()) {
- checkpointStorageView.initializeBaseLocationsForCheckpoint();
- }
Review comment:
Ok, I see. In that case I would summarise the discussion inside this
commit message, to give reasons why are you changing this code and that it
relates to/reimplements FLINK-23180.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/StateWithExecutionGraph.java
##########
@@ -221,6 +221,29 @@ void notifyKvStateUnregistered(
jobId, jobVertexId, keyGroupRange, registrationName);
}
+ CompletableFuture<String> triggerCheckpoint() {
Review comment:
nitty nit: put it below `triggerSavepoint` for the sake of consistency?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
