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 2021/06/15 12:35:43 UTC
[GitHub] [flink] zentol opened a new pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
zentol opened a new pull request #16165:
URL: https://github.com/apache/flink/pull/16165
introduces duration-based versions of `AkkaOptions#ASK_TIMEOUT` and `#LOOKUP_TIMEOUT`, and migrates all existing usages to these methods.
The goal is to reduce the dependency on `AkkaUtils` and places where custom parsing might occur.
--
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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
}, {
"hash" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320",
"triggerID" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"triggerType" : "PUSH"
}, {
"hash" : "60a84dedcd231d1b468483d0e7750d7b687f1c4a",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19397",
"triggerID" : "60a84dedcd231d1b468483d0e7750d7b687f1c4a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 48be6767ad83db5af8199bf2a300c2fc53d22a8b Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320)
* 60a84dedcd231d1b468483d0e7750d7b687f1c4a Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19397)
<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] nicoweidner commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
nicoweidner commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r657034096
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
ConfigOptions.key("akka.ask.timeout")
- .stringType()
- .defaultValue("10 s")
+ .durationType()
+ .defaultValue(Duration.ofSeconds(10))
.withDescription(
"Timeout used for all futures and blocking Akka calls. If Flink fails due to timeouts then you"
+ " should try to increase this value. Timeouts can be caused by slow machines or a congested network. The"
+ " timeout value requires a time-unit specifier (ms/s/min/h/d).");
Review comment:
I see, thanks!
--
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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 32a10c08b23c3d743b58c0ee3675df67357a6a04 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998)
<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] zentol commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r656938812
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
ConfigOptions.key("akka.ask.timeout")
- .stringType()
- .defaultValue("10 s")
+ .durationType()
+ .defaultValue(Duration.ofSeconds(10))
.withDescription(
"Timeout used for all futures and blocking Akka calls. If Flink fails due to timeouts then you"
+ " should try to increase this value. Timeouts can be caused by slow machines or a congested network. The"
+ " timeout value requires a time-unit specifier (ms/s/min/h/d).");
Review comment:
It comes into play when configuring this option via the flink-conf.yaml, where you'd specify something like:
```
akka.ask.timeout: 10s
```
This is then parsed via `org.apache.flink.util.TimeUtils#parseDuration`.
--
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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
}, {
"hash" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320",
"triggerID" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 48be6767ad83db5af8199bf2a300c2fc53d22a8b Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320)
<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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
}, {
"hash" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 32a10c08b23c3d743b58c0ee3675df67357a6a04 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998)
* 48be6767ad83db5af8199bf2a300c2fc53d22a8b 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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
}, {
"hash" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320",
"triggerID" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"triggerType" : "PUSH"
}, {
"hash" : "60a84dedcd231d1b468483d0e7750d7b687f1c4a",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "60a84dedcd231d1b468483d0e7750d7b687f1c4a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 48be6767ad83db5af8199bf2a300c2fc53d22a8b Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320)
* 60a84dedcd231d1b468483d0e7750d7b687f1c4a 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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
}, {
"hash" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320",
"triggerID" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 32a10c08b23c3d743b58c0ee3675df67357a6a04 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998)
* 48be6767ad83db5af8199bf2a300c2fc53d22a8b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320)
<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] zentol merged pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
zentol merged pull request #16165:
URL: https://github.com/apache/flink/pull/16165
--
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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861462190
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 32a10c08b23c3d743b58c0ee3675df67357a6a04 (Tue Jun 15 12:37:49 UTC 2021)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 32a10c08b23c3d743b58c0ee3675df67357a6a04 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] zentol commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r656899430
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
ConfigOptions.key("akka.ask.timeout")
- .stringType()
- .defaultValue("10 s")
+ .durationType()
+ .defaultValue(Duration.ofSeconds(10))
.withDescription(
"Timeout used for all futures and blocking Akka calls. If Flink fails due to timeouts then you"
+ " should try to increase this value. Timeouts can be caused by slow machines or a congested network. The"
+ " timeout value requires a time-unit specifier (ms/s/min/h/d).");
Review comment:
I think it is still required because the way we parse durations is still custom.
--
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] nicoweidner commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
nicoweidner commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r656822775
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
ConfigOptions.key("akka.ask.timeout")
- .stringType()
- .defaultValue("10 s")
+ .durationType()
+ .defaultValue(Duration.ofSeconds(10))
.withDescription(
"Timeout used for all futures and blocking Akka calls. If Flink fails due to timeouts then you"
+ " should try to increase this value. Timeouts can be caused by slow machines or a congested network. The"
+ " timeout value requires a time-unit specifier (ms/s/min/h/d).");
Review comment:
Is the mention of requiring a time-unit specifier still relevant? I feel this was important when it was string type, but it's automatically included if the type is `Duration`
(changing it would mean that the deprecated `ASK_TIMEOUT` should have a different description, though)
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -101,14 +114,23 @@
"Turns on the Akka’s remote logging of events. Set this value to 'true' in case of debugging.");
/** Timeout for all blocking calls that look up remote actors. */
- public static final ConfigOption<String> LOOKUP_TIMEOUT =
+ public static final ConfigOption<Duration> LOOKUP_TIMEOUT_DURATION =
ConfigOptions.key("akka.lookup.timeout")
- .stringType()
- .defaultValue("10 s")
+ .durationType()
+ .defaultValue(Duration.ofSeconds(10))
.withDescription(
"Timeout used for the lookup of the JobManager. The timeout value has to contain a time-unit"
+ " specifier (ms/s/min/h/d).");
Review comment:
Same point about the time-unit specifier as for `ASK_TIMEOUT`
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/TestingDefaultExecutionGraphBuilder.java
##########
@@ -62,10 +62,9 @@ public static TestingDefaultExecutionGraphBuilder newBuilder() {
private ScheduledExecutorService futureExecutor = TestingUtils.defaultExecutor();
private Executor ioExecutor = TestingUtils.defaultExecutor();
- private Time rpcTimeout = AkkaUtils.getDefaultTimeout();
+ private Time rpcTimeout = Time.fromDuration(AkkaOptions.ASK_TIMEOUT_DURATION.defaultValue());
private ClassLoader userClassLoader = DefaultExecutionGraph.class.getClassLoader();
private BlobWriter blobWriter = VoidBlobWriter.getInstance();
- private Time allocationTimeout = AkkaUtils.getDefaultTimeout();
Review comment:
Was this field simply unused?
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
Review comment:
Looking at the usages, I wondered why we use `Duration` in the first place instead of `Time`. I saw only two usages that actually use the Duration class. One is in `AkkaUtils` (and could be done with `Time` as well), the other in `WebMonitorEndpoint` (which I didn't follow further to see how it relies on `Duration`).
Is it because `Time` is marked `@PublicEvolving`, so potentially unstable?
--
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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
}, {
"hash" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19320",
"triggerID" : "48be6767ad83db5af8199bf2a300c2fc53d22a8b",
"triggerType" : "PUSH"
}, {
"hash" : "60a84dedcd231d1b468483d0e7750d7b687f1c4a",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19397",
"triggerID" : "60a84dedcd231d1b468483d0e7750d7b687f1c4a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 60a84dedcd231d1b468483d0e7750d7b687f1c4a Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=19397)
<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] zentol commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r656900838
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
Review comment:
We actually want to get rid of `Time` eventually because `Duration` provides the same functionality. FLINK-14068
--
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] zentol commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r656902472
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/TestingDefaultExecutionGraphBuilder.java
##########
@@ -62,10 +62,9 @@ public static TestingDefaultExecutionGraphBuilder newBuilder() {
private ScheduledExecutorService futureExecutor = TestingUtils.defaultExecutor();
private Executor ioExecutor = TestingUtils.defaultExecutor();
- private Time rpcTimeout = AkkaUtils.getDefaultTimeout();
+ private Time rpcTimeout = Time.fromDuration(AkkaOptions.ASK_TIMEOUT_DURATION.defaultValue());
private ClassLoader userClassLoader = DefaultExecutionGraph.class.getClassLoader();
private BlobWriter blobWriter = VoidBlobWriter.getInstance();
- private Time allocationTimeout = AkkaUtils.getDefaultTimeout();
Review comment:
Yes; probably when we introduce the declarative slot management because that invalidated the allocation timeout.
--
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] nicoweidner commented on a change in pull request #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
nicoweidner commented on a change in pull request #16165:
URL: https://github.com/apache/flink/pull/16165#discussion_r656913543
##########
File path: flink-core/src/main/java/org/apache/flink/configuration/AkkaOptions.java
##########
@@ -39,14 +42,24 @@
+ "memory footprint.");
/** Timeout for akka ask calls. */
- public static final ConfigOption<String> ASK_TIMEOUT =
+ public static final ConfigOption<Duration> ASK_TIMEOUT_DURATION =
ConfigOptions.key("akka.ask.timeout")
- .stringType()
- .defaultValue("10 s")
+ .durationType()
+ .defaultValue(Duration.ofSeconds(10))
.withDescription(
"Timeout used for all futures and blocking Akka calls. If Flink fails due to timeouts then you"
+ " should try to increase this value. Timeouts can be caused by slow machines or a congested network. The"
+ " timeout value requires a time-unit specifier (ms/s/min/h/d).");
Review comment:
But isn't the "timeout value" that is referenced in the description the value associated with this config key? Since its type is `Duration`, I don't see where the time-unit specifier comes into play now.
I also just checked the `toString` implementation of `Duration` and that looks completely different...
--
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 #16165: [FLINK-22997] Replace AkkaUtils#get*Timeout*
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #16165:
URL: https://github.com/apache/flink/pull/16165#issuecomment-861488292
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998",
"triggerID" : "32a10c08b23c3d743b58c0ee3675df67357a6a04",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 32a10c08b23c3d743b58c0ee3675df67357a6a04 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=18998)
<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