You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StefanRRichter <gi...@git.apache.org> on 2018/05/14 09:25:34 UTC
[GitHub] flink pull request #6006: [FLINK-9355][checkpointing] Simplify configuration...
GitHub user StefanRRichter opened a pull request:
https://github.com/apache/flink/pull/6006
[FLINK-9355][checkpointing] Simplify configuration of local recovery …
…to a simple on/off switch
## What is the purpose of the change
This PR makes the configuration of local recovery easier for the user and turns it into a simple on/off switch.
## Brief change log
- Config option for local recovery is just a boolean now.
## Verifying this change
This change is already covered by existing tests, such as *(please describe tests)*.
## 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)`: (yes)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes)
- 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)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/StefanRRichter/flink local-recovery-config
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/6006.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #6006
----
commit c27fe7c8103999343c6a73cc3103f4b27fd97b63
Author: Stefan Richter <s....@...>
Date: 2018-05-14T09:14:47Z
[FLINK-9355][checkpointing] Simplify configuration of local recovery to a simple on/off switch
----
---
[GitHub] flink issue #6006: [FLINK-9355][checkpointing] Simplify configuration of loc...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6006
CC @tillrohrmann
---
[GitHub] flink pull request #6006: [FLINK-9355][checkpointing] Simplify configuration...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/6006#discussion_r188000337
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java ---
@@ -78,13 +78,13 @@
private final long timerServiceShutdownTimeout;
- private final LocalRecoveryConfig.LocalRecoveryMode localRecoveryMode;
+ private final boolean localRecovery;
--- End diff --
Maybe we could rename this field to `isLocalRecoveryEnabled`
---
[GitHub] flink pull request #6006: [FLINK-9355][checkpointing] Simplify configuration...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6006#discussion_r188002995
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java ---
@@ -128,8 +128,8 @@ public InetAddress getTaskManagerAddress() {
return localRecoveryStateRootDirectories;
}
- public LocalRecoveryConfig.LocalRecoveryMode getLocalRecoveryMode() {
- return localRecoveryMode;
+ public boolean isLocalRecovery() {
--- End diff --
👍
---
[GitHub] flink issue #6006: [FLINK-9355][checkpointing] Simplify configuration of loc...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/6006
You have to regenerate the config docs, instructions can be found [here](https://github.com/apache/flink/blob/master/flink-docs/README.md).
---
[GitHub] flink issue #6006: [FLINK-9355][checkpointing] Simplify configuration of loc...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6006
@zentol thanks for pointing that out. Updated the PR.
---
[GitHub] flink pull request #6006: [FLINK-9355][checkpointing] Simplify configuration...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/6006#discussion_r188000256
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java ---
@@ -128,8 +128,8 @@ public InetAddress getTaskManagerAddress() {
return localRecoveryStateRootDirectories;
}
- public LocalRecoveryConfig.LocalRecoveryMode getLocalRecoveryMode() {
- return localRecoveryMode;
+ public boolean isLocalRecovery() {
--- End diff --
Maybe we could rename it to `isLocalRecoveryEnabled`
---
[GitHub] flink pull request #6006: [FLINK-9355][checkpointing] Simplify configuration...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/6006
---
[GitHub] flink issue #6006: [FLINK-9355][checkpointing] Simplify configuration of loc...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6006
Thanks for the reviews. Will merge this.
---
[GitHub] flink pull request #6006: [FLINK-9355][checkpointing] Simplify configuration...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6006#discussion_r188002868
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java ---
@@ -78,13 +78,13 @@
private final long timerServiceShutdownTimeout;
- private final LocalRecoveryConfig.LocalRecoveryMode localRecoveryMode;
+ private final boolean localRecovery;
--- End diff --
I would agree to `localRecoveryEnabled` because IIRC the `is` should not be part of boolean field names, only of the accessor method name.
---