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 2020/10/20 17:58:56 UTC

[GitHub] [flink] danbosnichill opened a new pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

danbosnichill opened a new pull request #13711:
URL: https://github.com/apache/flink/pull/13711


   ## What is the purpose of the change
   
   This is to speed up tests that spend unnecessary time sleeping.
   
   ## Brief change log
   
   - Add a RetryStrategy interface that encapsulates retry configuration details.
   - Support it in FuturesUtil.
   - Plumb interface to Minicluster utils.
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   - Added unit tests
   
   I could not get the test suite to fully pass locally.  I sent an email to the user group about it.
   
   ## 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/Mesos, 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
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     }, {
       "hash" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121",
       "triggerID" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "triggerType" : "PUSH"
     }, {
       "hash" : "aba275090e92c577fb19d98fb5019ee725421b62",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8197",
       "triggerID" : "aba275090e92c577fb19d98fb5019ee725421b62",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * c2976b53fc0f384b408bc49ae459bc3dc83afd10 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121) 
   * aba275090e92c577fb19d98fb5019ee725421b62 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8197) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6c8fb272a1859f797ccd5cf62cb9392d9b287e9b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 1f877b3d8220d0896d3ae433e20a855ae1481411 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] tillrohrmann commented on a change in pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #13711:
URL: https://github.com/apache/flink/pull/13711#discussion_r510221384



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/retriever/impl/RpcGatewayRetriever.java
##########
@@ -42,24 +44,26 @@
 	private final RpcService rpcService;
 	private final Class<T> gatewayType;
 	private final Function<UUID, F> fencingTokenMapper;
-
-	private final int retries;
-	private final Time retryDelay;
+	private final RetryStrategy retryStrategy;
 
 	public RpcGatewayRetriever(
 			RpcService rpcService,
 			Class<T> gatewayType,
 			Function<UUID, F> fencingTokenMapper,
 			int retries,
 			Time retryDelay) {
-		this.rpcService = Preconditions.checkNotNull(rpcService);
+		this(rpcService, gatewayType, fencingTokenMapper, new FixedRetryStrategy(retries, retryDelay));

Review comment:
       I think it should be fine to use an exponential backoff strategy with an initial timeout of `10ms` and with a max timeout of `50ms` and `12` attempts. I will try to do it based on your PR.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [flink] tillrohrmann closed pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
tillrohrmann closed pull request #13711:
URL: https://github.com/apache/flink/pull/13711


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     }, {
       "hash" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121",
       "triggerID" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "triggerType" : "PUSH"
     }, {
       "hash" : "aba275090e92c577fb19d98fb5019ee725421b62",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8197",
       "triggerID" : "aba275090e92c577fb19d98fb5019ee725421b62",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * aba275090e92c577fb19d98fb5019ee725421b62 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8197) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] danbosnichill commented on a change in pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
danbosnichill commented on a change in pull request #13711:
URL: https://github.com/apache/flink/pull/13711#discussion_r509814812



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/retriever/impl/RpcGatewayRetriever.java
##########
@@ -42,24 +44,26 @@
 	private final RpcService rpcService;
 	private final Class<T> gatewayType;
 	private final Function<UUID, F> fencingTokenMapper;
-
-	private final int retries;
-	private final Time retryDelay;
+	private final RetryStrategy retryStrategy;
 
 	public RpcGatewayRetriever(
 			RpcService rpcService,
 			Class<T> gatewayType,
 			Function<UUID, F> fencingTokenMapper,
 			int retries,
 			Time retryDelay) {
-		this.rpcService = Preconditions.checkNotNull(rpcService);
+		this(rpcService, gatewayType, fencingTokenMapper, new FixedRetryStrategy(retries, retryDelay));

Review comment:
       I don't know the code base well enough to make that decision.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6c8fb272a1859f797ccd5cf62cb9392d9b287e9b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963) 
   * 1f877b3d8220d0896d3ae433e20a855ae1481411 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot commented on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713038988


   Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
   to review your pull request. We will use this comment to track the progress of the review.
   
   
   ## Automated Checks
   Last check on commit 6c8fb272a1859f797ccd5cf62cb9392d9b287e9b (Tue Oct 20 18:01:44 UTC 2020)
   
   **Warnings:**
    * No documentation files were touched! Remember to keep the Flink docs up to date!
    * **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-19721).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
   
   
   <sub>Mention the bot in a comment to re-run the automated checks.</sub>
   ## Review Progress
   
   * ❓ 1. The [description] looks good.
   * ❓ 2. There is [consensus] that the contribution should go into to Flink.
   * ❓ 3. Needs [attention] from.
   * ❓ 4. The change fits into the overall [architecture].
   * ❓ 5. Overall code [quality] is good.
   
   Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
    The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
    - `@flinkbot approve all` to approve all aspects
    - `@flinkbot approve-until architecture` to approve everything until `architecture`
    - `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
    - `@flinkbot disapprove architecture` to remove an approval you gave earlier
   </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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot commented on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6c8fb272a1859f797ccd5cf62cb9392d9b287e9b UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] danbosnichill commented on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
danbosnichill commented on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-714186065


   I switched to `java.time.Duration`.
   
   I did not switch other calls to `ExponentialBackoffRetryStrategy`.  I do not know the code base well enough to say if this is worth doing.  E.g. what are safe minimums?  What frequency and backoff makes sense?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [flink] tillrohrmann commented on a change in pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #13711:
URL: https://github.com/apache/flink/pull/13711#discussion_r509064348



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/concurrent/ExponentialBackoffRetryStrategy.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.concurrent;
+
+import org.apache.flink.api.common.time.Time;
+import org.apache.flink.util.Preconditions;
+
+/**
+ * An implementation of {@link RetryStrategy} that retries that has an exponential backoff with
+ * a cap.
+ */
+public class ExponentialBackoffRetryStrategy implements RetryStrategy {
+	private final int remainingRetries;
+	private final Time currentRetryDelay;
+	private final Time maxRetryDelay;
+
+	/**
+	 * @param remainingRetries number of times to retry
+	 * @param currentRetryDelay the current delay between retries
+	 * @param maxRetryDelay the max delay between retries
+	 */
+	public ExponentialBackoffRetryStrategy(int remainingRetries, Time currentRetryDelay, Time maxRetryDelay) {

Review comment:
       I know that we use `Time` at many places but it is better now to use `java.time.Duration` because `Time` is actually an API class.

##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/retriever/impl/RpcGatewayRetriever.java
##########
@@ -42,24 +44,26 @@
 	private final RpcService rpcService;
 	private final Class<T> gatewayType;
 	private final Function<UUID, F> fencingTokenMapper;
-
-	private final int retries;
-	private final Time retryDelay;
+	private final RetryStrategy retryStrategy;
 
 	public RpcGatewayRetriever(
 			RpcService rpcService,
 			Class<T> gatewayType,
 			Function<UUID, F> fencingTokenMapper,
 			int retries,
 			Time retryDelay) {
-		this.rpcService = Preconditions.checkNotNull(rpcService);
+		this(rpcService, gatewayType, fencingTokenMapper, new FixedRetryStrategy(retries, retryDelay));

Review comment:
       I am wondering whether we shouldn't use the exponential backoff strategy by default.

##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/concurrent/RetryStrategy.java
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.concurrent;
+
+import org.apache.flink.api.common.time.Time;
+
+/**
+ * Interface that encapsulates retry logic.  An instances should be immutable.
+ */
+public interface RetryStrategy {
+	/**
+	 * @return the number of remaining retries
+	 */
+	int getNumRemainingRetries();
+
+	/**
+	 * @return the current delay if we need to retry
+	 */
+	Time getRetryDelay();

Review comment:
       ```suggestion
   	Duration getRetryDelay();
   ```

##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/concurrent/FixedRetryStrategy.java
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.concurrent;
+
+import org.apache.flink.api.common.time.Time;
+import org.apache.flink.util.Preconditions;
+
+/**
+ * An implementation of {@link RetryStrategy} that retries at a fixed delay.
+ */
+public class FixedRetryStrategy implements RetryStrategy {
+	private final int remainingRetries;
+	private final Time retryDelay;
+
+	/**
+	 * @param remainingRetries number of times to retry
+	 * @param retryDelay delay between retries
+	 */
+	public FixedRetryStrategy(int remainingRetries, Time retryDelay) {

Review comment:
       Same here with `Time`. I think it would be better to let the new components use `java.time.Duration`.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6c8fb272a1859f797ccd5cf62cb9392d9b287e9b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6c8fb272a1859f797ccd5cf62cb9392d9b287e9b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963) 
   * 1f877b3d8220d0896d3ae433e20a855ae1481411 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     }, {
       "hash" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 1f877b3d8220d0896d3ae433e20a855ae1481411 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059) 
   * c2976b53fc0f384b408bc49ae459bc3dc83afd10 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     }, {
       "hash" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121",
       "triggerID" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * c2976b53fc0f384b408bc49ae459bc3dc83afd10 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] flinkbot edited a comment on pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13711:
URL: https://github.com/apache/flink/pull/13711#issuecomment-713051309


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7963",
       "triggerID" : "6c8fb272a1859f797ccd5cf62cb9392d9b287e9b",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8059",
       "triggerID" : "1f877b3d8220d0896d3ae433e20a855ae1481411",
       "triggerType" : "PUSH"
     }, {
       "hash" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121",
       "triggerID" : "c2976b53fc0f384b408bc49ae459bc3dc83afd10",
       "triggerType" : "PUSH"
     }, {
       "hash" : "aba275090e92c577fb19d98fb5019ee725421b62",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "aba275090e92c577fb19d98fb5019ee725421b62",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * c2976b53fc0f384b408bc49ae459bc3dc83afd10 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8121) 
   * aba275090e92c577fb19d98fb5019ee725421b62 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink] danbosnichill commented on a change in pull request #13711: [FLINK-19721] [flink-runtime] Support exponential backoff retries in RpcGatewayRetriever

Posted by GitBox <gi...@apache.org>.
danbosnichill commented on a change in pull request #13711:
URL: https://github.com/apache/flink/pull/13711#discussion_r509814560



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/concurrent/ExponentialBackoffRetryStrategy.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.concurrent;
+
+import org.apache.flink.api.common.time.Time;
+import org.apache.flink.util.Preconditions;
+
+/**
+ * An implementation of {@link RetryStrategy} that retries that has an exponential backoff with
+ * a cap.
+ */
+public class ExponentialBackoffRetryStrategy implements RetryStrategy {
+	private final int remainingRetries;
+	private final Time currentRetryDelay;
+	private final Time maxRetryDelay;
+
+	/**
+	 * @param remainingRetries number of times to retry
+	 * @param currentRetryDelay the current delay between retries
+	 * @param maxRetryDelay the max delay between retries
+	 */
+	public ExponentialBackoffRetryStrategy(int remainingRetries, Time currentRetryDelay, Time maxRetryDelay) {

Review comment:
       Yea, I can change it.

##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/concurrent/FixedRetryStrategy.java
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.concurrent;
+
+import org.apache.flink.api.common.time.Time;
+import org.apache.flink.util.Preconditions;
+
+/**
+ * An implementation of {@link RetryStrategy} that retries at a fixed delay.
+ */
+public class FixedRetryStrategy implements RetryStrategy {
+	private final int remainingRetries;
+	private final Time retryDelay;
+
+	/**
+	 * @param remainingRetries number of times to retry
+	 * @param retryDelay delay between retries
+	 */
+	public FixedRetryStrategy(int remainingRetries, Time retryDelay) {

Review comment:
       Ditto.




----------------------------------------------------------------
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:
users@infra.apache.org