You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by rehevkor5 <gi...@git.apache.org> on 2017/01/04 19:21:33 UTC
[GitHub] flink pull request #3059: [docs] Clarify restart strategy defaults set by ch...
GitHub user rehevkor5 opened a pull request:
https://github.com/apache/flink/pull/3059
[docs] Clarify restart strategy defaults set by checkpointing
- Added info about checkpointing changing the default restart
strategy in places where it was missing: the config page and the
section about the fixed-delay strategy
- Replaced no-restart with "no restart" so people don't think we're
referring to a config value
- Replaced invalid <it> html tag with <code>
- Fixed bad link to restart strategies page from state.md
Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful description of your changes.
- [ ] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [ ] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [ ] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rehevkor5/flink clarify_retry_strategy_defaults
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3059.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 #3059
----
commit e7bba190a4d14e5e098acc077856a69839bc7d88
Author: Shannon Carey <re...@gmail.com>
Date: 2017-01-04T19:20:12Z
[docs] Clarify restart strategy defaults set by checkpointing
- Added info about checkpointing changing the default restart
strategy in places where it was missing: the config page and the
section about the fixed-delay strategy
- Replaced no-restart with "no restart" so people don't think we're
referring to a config value
- Replaced invalid <it> html tag with <code>
- Fixed bad link to restart strategies page from state.md
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by rehevkor5 <gi...@git.apache.org>.
Github user rehevkor5 commented on the issue:
https://github.com/apache/flink/pull/3059
The changes so far are docs-only, so the CI failure must be unrelated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3059: [docs] Clarify restart strategy defaults set by ch...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3059
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:
https://github.com/apache/flink/pull/3059
Thanks for these clarifications. I'm going to merge it with the next batch. In the current master we also have a new checkpoints page at `setup/checkpoints.html` where we can add these notes as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:
https://github.com/apache/flink/pull/3059
Since the comments were very minor I addressed them myself (used the backticks and skipped the change in "If checkpointing is activated..." as it didn't really improve anything).
The work flow was as follows: I checked out your PR branch, rebased it onto master (via `git rebase apache/master`) and pushed it with another related commit. My related commit had a comment `This closes #3059`, which tells the ASF bot to close this PR. Then I cherry picked it to the 1.2 branch.
GitHub only notices that the changes were merged for merges to master and if the commit hashes are not changed I think. Since I rebased they did change and I needed the manual close. Does this make sense?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by rehevkor5 <gi...@git.apache.org>.
Github user rehevkor5 commented on the issue:
https://github.com/apache/flink/pull/3059
I'm curious as to how you added my commit to `master` and `release-1.2` but it doesn't show up in this PR. The whole "rehevkor5 committed with uce 5 days ago" thing. Github isn't aware that the changes in this PR were merged.
If you have a moment can you describe your workflow is so I can understand what's happening behind the scenes? Is it rebase onto master + merge --ff-only or something? Maybe it's due to the fact that Github is only mirroring another repo? I'd appreciate it!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3059: [docs] Clarify restart strategy defaults set by ch...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/3059#discussion_r95131686
--- Diff: docs/dev/restart_strategies.md ---
@@ -28,8 +28,9 @@ In case that the job is submitted with a restart strategy, this strategy overrid
The default restart strategy is set via Flink's configuration file `flink-conf.yaml`.
The configuration parameter *restart-strategy* defines which strategy is taken.
-Per default, the no-restart strategy is used.
-When checkpointing is activated and no restart strategy has been configured, the job will be restarted infinitely often.
+If checkpointing is not enabled, the "no restart" strategy is used.
+If checkpointing is activated and the restart strategy has not been configured, the fixed-delay strategy is used with
--- End diff --
Should we say "If checkpointing is activated and no restart strategy has been explicitly configured, ..."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by rehevkor5 <gi...@git.apache.org>.
Github user rehevkor5 commented on the issue:
https://github.com/apache/flink/pull/3059
@uce Thanks. I didn't get an opportunity to address your comments... is there anything you want me to do?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3059: [docs] Clarify restart strategy defaults set by ch...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/3059#discussion_r95131517
--- Diff: docs/setup/config.md ---
@@ -162,24 +162,27 @@ will be used under the directory specified by jobmanager.web.tmpdir.
- `high-availability.zookeeper.storageDir`: Required for HA. Directory for storing JobManager metadata; this is persisted in the state backend and only a pointer to this state is stored in ZooKeeper. Exactly like the checkpoint directory it must be accessible from the JobManager and a local filesystem should only be used for local deployments. Previously this key was named `recovery.zookeeper.storageDir`.
-- `blob.storage.directory`: Directory for storing blobs (such as user jar's) on the TaskManagers.
+- `blob.storage.directory`: Directory for storing blobs (such as user JARs) on the TaskManagers.
-- `blob.server.port`: Port definition for the blob server (serving user jar's) on the Taskmanagers. By default the port is set to 0, which means that the operating system is picking an ephemeral port. Flink also accepts a list of ports ("50100,50101"), ranges ("50100-50200") or a combination of both. It is recommended to set a range of ports to avoid collisions when multiple JobManagers are running on the same machine.
+- `blob.server.port`: Port definition for the blob server (serving user JARs) on the TaskManagers. By default the port is set to 0, which means that the operating system is picking an ephemeral port. Flink also accepts a list of ports ("50100,50101"), ranges ("50100-50200") or a combination of both. It is recommended to set a range of ports to avoid collisions when multiple JobManagers are running on the same machine.
- `blob.service.ssl.enabled`: Flag to enable ssl for the blob client/server communication. This is applicable only when the global ssl flag security.ssl.enabled is set to true (DEFAULT: true).
-- `restart-strategy`: Default restart strategy to use in case that no restart strategy has been specified for the submitted job.
-Currently, it can be chosen from fixed delay restart strategy, failure rate restart strategy or no restart strategy.
-To use the fixed delay strategy you have to specify "fixed-delay".
-To use the failure rate strategy you have to specify "failure-rate".
-To turn the restart behaviour off you have to specify "none".
-Default value "none".
+- `restart-strategy`: Default [restart strategy]({{site.baseurl}}/dev/restart_strategies.html) to use in case no
+restart strategy has been specified for the job.
+The options are:
+ - fixed delay strategy: "fixed-delay".
--- End diff --
Should we use code formatting with backticks instead of " for these as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by rehevkor5 <gi...@git.apache.org>.
Github user rehevkor5 commented on the issue:
https://github.com/apache/flink/pull/3059
It does, thanks! I'll try to remember to include "closes #_{pr number}_" in my commit messages which might be handy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3059: [docs] Clarify restart strategy defaults set by checkpoin...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:
https://github.com/apache/flink/pull/3059
Thanks again. Just merged this to `master` (b03ad79) and `release-1.2` (2aaa093).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---