You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/06/29 12:45:30 UTC
[GitHub] [flink] zoltar9264 opened a new pull request, #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
zoltar9264 opened a new pull request, #20103:
URL: https://github.com/apache/flink/pull/20103
…r changelog is enabled in the UI
## What is the purpose of the change
Show the delegated StateBackend and whether changelog is enabled in the UI, described in [FLINK-28178](https://issues.apache.org/jira/browse/FLINK-28178).
## Brief change log
- set field 'state_backend' in /jobs/:jobid/checkpoints/config to delegated StateBackend
- /jobs/:jobid/checkpoints/config add field 'state_changelog', which indicates whether the State Changelog enabled
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1170931628
Thanks a lot for your reminding @fredia ! I have push a commit fix 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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1204372419
Sorry for the delay @zoltar9264 .
I still don't see the changes when running locally (commit SHA shown in the UI: 7259856).
In the logs, I see messages like this:
```
2022-08-03 21:14:19,963 INFO org.apache.flink.state.changelog.ChangelogKeyedStateBackend [] - snapshot of SlidingWindowCheckMapper -> Sink: Sl...
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r923989858
##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -551,15 +551,15 @@
</td>
</tr>
<tr>
- <td>Changelog state-backend</td>
- <td *ngIf="checkPointConfig['state_backend'] === 'ChangelogStateBackend'">Enabled</td>
- <td *ngIf="checkPointConfig['state_backend'] !== 'ChangelogStateBackend'">Disabled</td>
+ <td>State Changelog</td>
Review Comment:
Hi @Myasuka , how do you think about “Changelog of Flink State” ?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1195070679
Sorry @rkhachatryan , you are right. I test it on yarn cluster before and it works. I test it locally just now, it really does't work. I will check 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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1209286935
Thanks @rkhachatryan !
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1175011583
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1170843654
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920699386
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ArchivedExecutionGraph.java:
##########
@@ -391,6 +400,7 @@ public static ArchivedExecutionGraph createSparseArchivedExecutionGraph(
checkpointingSettings == null ? null : CheckpointStatsSnapshot.empty(),
checkpointingSettings == null ? null : "Unknown",
checkpointingSettings == null ? null : "Unknown",
+ false,
Review Comment:
Sorry @Myasuka , It's my cursoriness.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] fredia commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
fredia commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r927635877
##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -551,15 +551,15 @@
</td>
</tr>
<tr>
- <td>Changelog state-backend</td>
- <td *ngIf="checkPointConfig['state_backend'] === 'ChangelogStateBackend'">Enabled</td>
- <td *ngIf="checkPointConfig['state_backend'] !== 'ChangelogStateBackend'">Disabled</td>
+ <td>State Changelog</td>
+ <td *ngIf="checkPointConfig['state_changelog_enabled']">Enabled</td>
Review Comment:
Do we need to add this item into `CheckpointConfig` in `job-checkpoint.ts`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Myasuka commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920098039
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/AccessExecutionGraph.java:
##########
@@ -176,6 +176,13 @@ public interface AccessExecutionGraph extends JobStatusProvider {
*/
Optional<String> getCheckpointStorageName();
+ /**
+ * Returns whether the state changelog is enabled for this ExecutionGraph.
+ *
+ * @return true, if state changelog enabled, false otherwise.
+ */
+ boolean isStateChangelogEnabled();
Review Comment:
I think `isChangelogStateBackendEnabled` looks better.
##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -551,15 +551,15 @@
</td>
</tr>
<tr>
- <td>Changelog state-backend</td>
- <td *ngIf="checkPointConfig['state_backend'] === 'ChangelogStateBackend'">Enabled</td>
- <td *ngIf="checkPointConfig['state_backend'] !== 'ChangelogStateBackend'">Disabled</td>
+ <td>State Changelog</td>
Review Comment:
```suggestion
<td>Changelog State</td>
```
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointConfigInfo.java:
##########
@@ -65,6 +65,8 @@ public class CheckpointConfigInfo implements ResponseBody {
public static final String FIELD_NAME_CHECKPOINTS_AFTER_TASKS_FINISH =
"checkpoints_after_tasks_finish";
+ public static final String FIELD_NAME_STATE_CHANGELOG = "state_changelog";
Review Comment:
I think `changelog_state_enabled` looks better.
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ArchivedExecutionGraph.java:
##########
@@ -391,6 +400,7 @@ public static ArchivedExecutionGraph createSparseArchivedExecutionGraph(
checkpointingSettings == null ? null : CheckpointStatsSnapshot.empty(),
checkpointingSettings == null ? null : "Unknown",
checkpointingSettings == null ? null : "Unknown",
+ false,
Review Comment:
This would always be `false`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1209275667
The build [succeeded](https://dev.azure.com/khachatryanroman/flink/_build/results?buildId=1601&view=results) in a private branch, merging.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920678551
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointConfigInfo.java:
##########
@@ -65,6 +65,8 @@ public class CheckpointConfigInfo implements ResponseBody {
public static final String FIELD_NAME_CHECKPOINTS_AFTER_TASKS_FINISH =
"checkpoints_after_tasks_finish";
+ public static final String FIELD_NAME_STATE_CHANGELOG = "state_changelog";
Review Comment:
How about 'state_changelog_enabled' ?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1192408479
I was trying to test the changes locally.
The UI shows commit `8788d51` but no changelog configuration shown. I do see changelog is used in the logs.
@zoltar9264 could you please verify?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan merged pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan merged PR #20103:
URL: https://github.com/apache/flink/pull/20103
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r940970843
##########
docs/layouts/shortcodes/generated/rest_v1_dispatcher.html:
##########
@@ -2335,6 +2335,9 @@
"state_backend" : {
"type" : "string"
},
+ "state_changelog_enabled" : {
+ "type" : "boolean"
+ },
Review Comment:
Hi @rkhachatryan , here is [#20103](https://github.com/apache/flink/pull/20103) .
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1209013006
Thanks @rkhachatryan , I have squash all commit into one.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1169940664
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5673fff99ff4e0ab0ce24550e060524c1519ed6f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5673fff99ff4e0ab0ce24550e060524c1519ed6f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5673fff99ff4e0ab0ce24550e060524c1519ed6f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920675336
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/AccessExecutionGraph.java:
##########
@@ -176,6 +176,13 @@ public interface AccessExecutionGraph extends JobStatusProvider {
*/
Optional<String> getCheckpointStorageName();
+ /**
+ * Returns whether the state changelog is enabled for this ExecutionGraph.
+ *
+ * @return true, if state changelog enabled, false otherwise.
+ */
+ boolean isStateChangelogEnabled();
Review Comment:
Hi @Myasuka , thanks for the suggession .I think ’State Changelog‘ is more suitable to describe this feature, and ’ChangelogStateBackend‘ is the specific implementation of it. WDYT ?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1170933814
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1202025354
Kindly ping @Myasuka @rkhachatryan .
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1209043262
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r923988243
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointConfigInfo.java:
##########
@@ -65,6 +65,8 @@ public class CheckpointConfigInfo implements ResponseBody {
public static final String FIELD_NAME_CHECKPOINTS_AFTER_TASKS_FINISH =
"checkpoints_after_tasks_finish";
+ public static final String FIELD_NAME_STATE_CHANGELOG = "state_changelog";
Review Comment:
Hi @Myasuka , I change it to 'state_changelog_enabled'.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1195101790
Hi @rkhachatryan , I fixed this with yanfei's tip. And I test it locally, it works now.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r921009823
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/AccessExecutionGraph.java:
##########
@@ -176,6 +176,13 @@ public interface AccessExecutionGraph extends JobStatusProvider {
*/
Optional<String> getCheckpointStorageName();
+ /**
+ * Returns whether the state changelog is enabled for this ExecutionGraph.
+ *
+ * @return true, if state changelog enabled, false otherwise.
+ */
+ boolean isStateChangelogEnabled();
Review Comment:
To me, making it consistent with the other parts of code makes sense (so `isChangelogStateBackendEnabled` seems preferrable); otherwise, I don't have a strong opinion on this.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920700958
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/AccessExecutionGraph.java:
##########
@@ -176,6 +176,13 @@ public interface AccessExecutionGraph extends JobStatusProvider {
*/
Optional<String> getCheckpointStorageName();
+ /**
+ * Returns whether the state changelog is enabled for this ExecutionGraph.
+ *
+ * @return true, if state changelog enabled, false otherwise.
+ */
+ boolean isStateChangelogEnabled();
Review Comment:
I think isStateChangelogEnabled is easier to understand, but I've also noticed that isChangelogStateBackendEnabled has been used elsewhere. I don't know how to choose, or is my understanding simply wrong?
cc @rkhachatryan
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920699386
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ArchivedExecutionGraph.java:
##########
@@ -391,6 +400,7 @@ public static ArchivedExecutionGraph createSparseArchivedExecutionGraph(
checkpointingSettings == null ? null : CheckpointStatsSnapshot.empty(),
checkpointingSettings == null ? null : "Unknown",
checkpointingSettings == null ? null : "Unknown",
+ false,
Review Comment:
Sorry @Myasuka , It's my cursoriness. I have updated.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r921014688
##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -551,15 +551,15 @@
</td>
</tr>
<tr>
- <td>Changelog state-backend</td>
- <td *ngIf="checkPointConfig['state_backend'] === 'ChangelogStateBackend'">Enabled</td>
- <td *ngIf="checkPointConfig['state_backend'] !== 'ChangelogStateBackend'">Disabled</td>
+ <td>State Changelog</td>
Review Comment:
I think it should actually be `State Changelog` - i.e. "Changelog of Flink State";
`Changelog State` reads as "state of changelog of something".
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r921127174
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/AccessExecutionGraph.java:
##########
@@ -176,6 +176,13 @@ public interface AccessExecutionGraph extends JobStatusProvider {
*/
Optional<String> getCheckpointStorageName();
+ /**
+ * Returns whether the state changelog is enabled for this ExecutionGraph.
+ *
+ * @return true, if state changelog enabled, false otherwise.
+ */
+ boolean isStateChangelogEnabled();
Review Comment:
Actually I think all place where `isChangelogStateBackendEnabled` appeared should be `isStateChangelogEnabled`. But since these methods are not public API for users , I can also accept `isChangelogStateBackendEnabled` for consistence.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rkhachatryan commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r937093155
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/DefaultExecutionGraph.java:
##########
@@ -517,7 +527,16 @@ public void failJobDueToTaskFailure(
registerJobStatusListener(checkpointCoordinator.createActivatorDeactivator());
}
- this.stateBackendName = checkpointStateBackend.getClass().getSimpleName();
+ if (StateBackendLoader.isChangelogStateBackend(checkpointStateBackend)) {
+ this.stateChangelogEnabled = TernaryBoolean.TRUE;
+ StateBackend delegatedStateBackend =
+ ((DelegatingStateBackend) checkpointStateBackend).getDelegatedStateBackend();
+ this.stateBackendName = delegatedStateBackend.getClass().getSimpleName();
+ } else {
+ this.stateChangelogEnabled = TernaryBoolean.FALSE;
+ this.stateBackendName = checkpointStateBackend.getClass().getSimpleName();
+ }
+
Review Comment:
Can we replace `if` with class-dependent logic here?
For example
```
this.stateBackendName = checkpointStateBackend.getName();
this.stateChangelogEnabled =
TernaryBoolean.fromBoolean(
StateBackendLoader.isChangelogStateBackend(checkpointStateBackend));
```
Where `getName()` by default returns `this.getClass().getSimpleName()`,
and changelog returns `delegatedStateBackend.getName()`?
##########
docs/layouts/shortcodes/generated/rest_v1_dispatcher.html:
##########
@@ -2335,6 +2335,9 @@
"state_backend" : {
"type" : "string"
},
+ "state_changelog_enabled" : {
+ "type" : "boolean"
+ },
Review Comment:
Is this actually related to this PR? Or #20103?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] fredia commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
fredia commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1170885957
Thanks for creating this PR @zoltar9264, It looks like you added `state_changelog` to rest api, could you please update the rest api doc? See [rest-api-documentation](https://github.com/apache/flink/tree/master/flink-docs#rest-api-documentation) for details.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r920700958
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/AccessExecutionGraph.java:
##########
@@ -176,6 +176,13 @@ public interface AccessExecutionGraph extends JobStatusProvider {
*/
Optional<String> getCheckpointStorageName();
+ /**
+ * Returns whether the state changelog is enabled for this ExecutionGraph.
+ *
+ * @return true, if state changelog enabled, false otherwise.
+ */
+ boolean isStateChangelogEnabled();
Review Comment:
I think isStateChangelogEnabled is easier to understand, but I've also noticed that isChangelogStateBackendEnabled has been used elsewhere. I don't know how to choose, or is my understanding simply wrong?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r929606940
##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -551,15 +551,15 @@
</td>
</tr>
<tr>
- <td>Changelog state-backend</td>
- <td *ngIf="checkPointConfig['state_backend'] === 'ChangelogStateBackend'">Enabled</td>
- <td *ngIf="checkPointConfig['state_backend'] !== 'ChangelogStateBackend'">Disabled</td>
+ <td>State Changelog</td>
+ <td *ngIf="checkPointConfig['state_changelog_enabled']">Enabled</td>
Review Comment:
Thanks @fredia for your reminder !
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #20103:
URL: https://github.com/apache/flink/pull/20103#issuecomment-1209096097
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zoltar9264 commented on a diff in pull request #20103: [FLINK-28178][runtime-web] Show the delegated StateBackend and whethe…
Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #20103:
URL: https://github.com/apache/flink/pull/20103#discussion_r940969237
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/DefaultExecutionGraph.java:
##########
@@ -517,7 +527,16 @@ public void failJobDueToTaskFailure(
registerJobStatusListener(checkpointCoordinator.createActivatorDeactivator());
}
- this.stateBackendName = checkpointStateBackend.getClass().getSimpleName();
+ if (StateBackendLoader.isChangelogStateBackend(checkpointStateBackend)) {
+ this.stateChangelogEnabled = TernaryBoolean.TRUE;
+ StateBackend delegatedStateBackend =
+ ((DelegatingStateBackend) checkpointStateBackend).getDelegatedStateBackend();
+ this.stateBackendName = delegatedStateBackend.getClass().getSimpleName();
+ } else {
+ this.stateChangelogEnabled = TernaryBoolean.FALSE;
+ this.stateBackendName = checkpointStateBackend.getClass().getSimpleName();
+ }
+
Review Comment:
Thanks @rkhachatryan , good idea, I have modified as your suggestion.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org