[GitHub] [flink] klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api URL: https://github.com/apache/flink/pull/10344#discussion_r352644494 ## File path: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/tasks/CheckpointCoordinatorConfiguration.java ## @@ -36,6 +38,8 @@ private static final long serialVersionUID = 2L; + private String stateBackendName; Review comment: Ahh, got it. Maybe we should get the `stateBackendName` from `AccessExecutionGraph` directly(add a new function `getStateBackend` in `AccessExecutionGraph`, pass the `stateBackendName` into `ArchivedExecutionGraph` when init a new `ArchivedExecutionGraph`), what do you think? 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
[GitHub] [flink] klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api URL: https://github.com/apache/flink/pull/10344#discussion_r352405508 ## File path: flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/job/checkpoints/CheckpointConfigHandler.java ## @@ -100,13 +100,15 @@ private static CheckpointConfigInfo createCheckpointConfigInfo(AccessExecutionGr retentionPolicy != CheckpointRetentionPolicy.NEVER_RETAIN_AFTER_TERMINATION, retentionPolicy != CheckpointRetentionPolicy.RETAIN_ON_CANCELLATION); + String stateName = checkpointCoordinatorConfiguration.getStateBackendName() == null ? "TEMPORARY_UNKNOWN" : checkpointCoordinatorConfiguration.getStateBackendName(); Review comment: @zentol Thanks for the clarification. Before I submit the PR, I tried a few times to query the rest before job submitted, but the job submitting duration holds too small, all end up with a running job. Maybe we should add a `checkNotNull` here for `stateBackendName`, what do you think. 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
[GitHub] [flink] klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
klion26 commented on a change in pull request #10344:
[FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
URL: https://github.com/apache/flink/pull/10344#discussion_r352404106
##
File path: docs/_includes/generated/rest_v1_dispatcher.html
##
@@ -2526,7 +2529,7 @@
"type" : "array",
"items" : {
"type" : "object",
-"id" :
"urn:jsonschema:org:apache:flink:runtime:rest:messages:SubtaskExecutionAttemptDetailsInfo",
+"id" :
"urn:jsonschema:org:apache:flink:runtime:rest:messages:job:SubtaskExecutionAttemptDetailsInfo",
Review comment:
ok, will update it
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
[GitHub] [flink] klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api URL: https://github.com/apache/flink/pull/10344#discussion_r352404077 ## File path: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/tasks/CheckpointCoordinatorConfiguration.java ## @@ -36,6 +38,8 @@ private static final long serialVersionUID = 2L; + private String stateBackendName; Review comment: I think it's the name here that makes people confused. the `stateBackendName` here is `checkpoint backend name`, such as `MemoryStateBackend`, `FsStateBackend` or `RocksDBStateBackend`, and I think they'are relevant with `CheckpointCoordinator`, we'll also pass the backend into [CheckpointCoordinator](https://github.com/apache/flink/blob/8c6cc4505a4c27daadb00cd94df8a7e955eb8d52/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java#L231) to init the checkpoint directory. Maybe we could update the name here, what do you think? 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
[GitHub] [flink] klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
klion26 commented on a change in pull request #10344:
[FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
URL: https://github.com/apache/flink/pull/10344#discussion_r351615345
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/tasks/CheckpointCoordinatorConfiguration.java
##
@@ -119,6 +123,14 @@ public int getTolerableCheckpointFailureNumber() {
return tolerableCheckpointFailureNumber;
}
+ public String getStateBackendName() {
+ return stateBackendName;
+ }
+
+ public void setStateBackendName(@Nonnull String newStateBackend) {
Review comment:
Add a setter function here instead of passed in from constructor, because
[`StreamGraph#getStateBackend()`](https://github.com/apache/flink/blob/f38106582e69765677ef8ebddece3a92641f6f22/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGenerator.java#L895)
may return null in `StreamingJobGraphGenerator#configureCheckpointing`
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
[GitHub] [flink] klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api
klion26 commented on a change in pull request #10344: [FLINK-14264][StateBackend][Rest] Expose state backend in checkpoint rest api URL: https://github.com/apache/flink/pull/10344#discussion_r351614505 ## File path: flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/job/checkpoints/CheckpointConfigHandler.java ## @@ -100,13 +100,15 @@ private static CheckpointConfigInfo createCheckpointConfigInfo(AccessExecutionGr retentionPolicy != CheckpointRetentionPolicy.NEVER_RETAIN_AFTER_TERMINATION, retentionPolicy != CheckpointRetentionPolicy.RETAIN_ON_CANCELLATION); + String stateName = checkpointCoordinatorConfiguration.getStateBackendName() == null ? "TEMPORARY_UNKNOWN" : checkpointCoordinatorConfiguration.getStateBackendName(); Review comment: If we query the rest before the job is running, maybe the `checkpointCoordinatorConfiguration.getStateBackendName()` will return null (please see the above comments), so transfer to `TEMPORARY_UNKNOWN` here. 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
