[FLINK-3410] [restart] Choose NoRestart strategy if the number of retries is set to 0 #1643
+106
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ries is set to 0 Add test case
| if (executionRetryDelay < 0 ) { | ||
| throw new IllegalArgumentException( | ||
| "The delay between reties must be non-negative, or -1 (use system default)"); | ||
| "The delay between reties must be non-negative."); |
Contributor
There was a problem hiding this comment.
we can fix this typo while we're at it: reties:retries
Contributor
Author
There was a problem hiding this comment.
Good catch. Overlooked the typo. Will fix it.
Contributor
Author
|
Thanks for the review @zentol. Will merge it. |
tillrohrmann
added a commit
to tillrohrmann/flink
that referenced
this pull request
Feb 23, 2016
…ries is set to 0 Add test case This closes apache#1643.
subhankarb
pushed a commit
to subhankarb/flink
that referenced
this pull request
Mar 17, 2016
…ries is set to 0 Add test case This closes apache#1643.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the problem that when checkpointing is enabled and the number of execution retries is set to
0that the automatic restarting should be deactivated. This is consistent with the semantics before the restart strategies where introduced.Be aware, though, that whenever you enable checkpointing for streaming jobs, the cluster wide default restart strategy which is set in the configuration will always be overwritten. Either by manually setting a restart strategy or by automatically setting a
FixedDelayRestartStrategy(Integer.MAX_VALUE, 10000)strategy in theStreamingJobGraphGeneratorif nothing was specified. That is consistent with the previous behaviour where all default execution retry attempts set in the configuration where overwritten in case of a checkpointed streaming job.Additionally, this PR sets the default retry delay to 10000 ms and disallows to set it to negative values. Before, the default execution retry delay would have been used if the delay in the
ExecutionConfigwas set to-1. However, with the newRestartStrategiesthere is no longer the possibility to set an explicit execution retry delay independent of the number of retries in the configuration file. Therefore it is no longer possible to set the number of execution retries in the configuration file and to specify the execution retry delay in theExecutionConfigor vice versa. Both have to be defined either in theExecutionConfigor the configuration file.