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 2022/02/11 15:55:27 UTC
[GitHub] [flink] CrynetLogistics opened a new pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
CrynetLogistics opened a new pull request #18733:
URL: https://github.com/apache/flink/pull/18733
… for async clients now
<!--
*Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
*Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.*
## Contribution Checklist
- Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue.
- Name the pull request in the form "[FLINK-XXXX] [component] Title of the pull request", where *FLINK-XXXX* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component.
Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Azure Pipelines CI to do that following [this guide](https://cwiki.apache.org/confluence/display/FLINK/Azure+Pipelines#AzurePipelines-Tutorial:SettingupAzurePipelinesforaforkoftheFlinkrepository).
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message (including the JIRA id)
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
## What is the purpose of the change
Introduces own event loop group whenever we create an http client for the aws sdk clients. The issue was found [here](https://github.com/apache/flink/pull/18553#issuecomment-1035369952).
Also reverts the `Ignoring` of the Firehose [integration test cases](https://github.com/apache/flink/pull/18712).
## Brief change log
Introduces new event loop group on creation of http client in aws base utility class.
## Verifying this change
Added unit test to verify usage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / no)n
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / no)n
- The serializers: (yes / no / don't know)n
- The runtime per-record code paths (performance sensitive): (yes / no / don't know)y
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know)n
- The S3 file system connector: (yes / no / don't know)n
## Documentation
- Does this pull request introduce a new feature? (yes / no)n
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)n
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r805842675
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
Hey, thank you for asking.
So it defaults to 0 threads and calls:
https://github.com/aws/aws-sdk-java-v2/blob/52a737b464d6d91247f1c53cfc994eeaeb375b5f/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/SdkEventLoopGroup.java#L132
From here, you can see it calls `super` with 0 threads too:
https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/channel/nio/NioEventLoopGroup.java
Finally in `MultithreadEventLoopGroup`, we have:
https://github.com/netty/netty/blob/d1a8cd2dec422f3b8fbfd186ae1ec29e680ffd4e/transport/src/main/java/io/netty/channel/MultithreadEventLoopGroup.java#L59
Which checks if threads=0 and if so uses the following formula:
https://github.com/netty/netty/blob/d1a8cd2dec422f3b8fbfd186ae1ec29e680ffd4e/transport/src/main/java/io/netty/channel/MultithreadEventLoopGroup.java#L40
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r805143370
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -274,6 +275,7 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
httpClientBuilder
+ .eventLoopGroupBuilder(SdkEventLoopGroup.builder())
Review comment:
Thanks for adding a test to verify this too 👍
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1042972664
If there are two jobs running on the same TM and they're both using the shared ELG, how comes they can't interfere with each other?
In general I'm inclined to use the shared reference counted ELG, but we'd probably have to scope it per jobId.
I have some time for the discussion today and then I'm off until Monday.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r804791218
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -274,6 +275,7 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
httpClientBuilder
+ .eventLoopGroupBuilder(SdkEventLoopGroup.builder())
Review comment:
Looks like it is closed by httpClient's `.close` whether you provide a builder or not.
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -274,6 +275,7 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
httpClientBuilder
+ .eventLoopGroupBuilder(SdkEventLoopGroup.builder())
Review comment:
Despite this in the [JavaDocs](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.Builder.html#:~:text=The%20EventLoopGroup%20MUST%20be%20closed%20by%20the%20caller%20when%20it%20is%20ready%20to%20be%20disposed.)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1040593983
I have been taking a deeper dive into this issue and am no longer inclined to make this change. I have verified that when running on a cluster, each job creates it's own `aws-java-sdk-NettyEventLoop` threads, and jobs do not interfere with each other. Running test cluster with 1 TM 1 CPU and 2 slots, posting 2 identical jobs. First job produces 2 threads:
```
"aws-java-sdk-NettyEventLoop-0-0" daemon prio=5 Id=76 RUNNABLE
"aws-java-sdk-NettyEventLoop-0-1" daemon prio=5 Id=77 RUNNABLE
```
Second job produces 2 threads:
```
"aws-java-sdk-NettyEventLoop-0-0" daemon prio=5 Id=95 RUNNABLE
"aws-java-sdk-NettyEventLoop-0-1" daemon prio=5 Id=96 RUNNABLE
```
Third job produces more threads etc. Killing jobs does not impact the other running jobs. You will notice the thread names are the same. This is because they are isolated in different job classloaders and cannot see each other to [increment the pool number](https://github.com/aws/aws-sdk-java-v2/blob/master/utils/src/main/java/software/amazon/awssdk/utils/ThreadFactoryBuilder.java#L75).
I have also managed to consistently reproduce this issue locally, and backup @dmvk findings. To reproduce this I made the following changes ([My test code](https://github.com/dannycranmer/flink/commit/6e42d2a052ca1ef2d13679159de9f9ad617121ac)):
- Switch the Flink job submission to `executeAsync()`
- Move the test client initialisation to after job submission
The problem in the flaky tests is that the `FlinkUserClassLoader` has the `Launcher$AppClassLoader` as it's parent classloader. This is the same classloader used by the test code. This means that both test code and Flink job can "see" the same ELG. As @dmvk pointed out, the Netty thread inherits current thread context classloader. The issue seems be be a race condition, since these clients are async, I guess that whoever makes the request first creates the Netty thread (not yet confirmed). The test blocks for Flink job to finish, which destroys the `FlinkUserClassLoader` and results in the classloader leak on the Netty threads.
Based on this, I am not inclined to create a new ELG per sub task, since this will potentially create a large resource overhead. This would also impact our current Kinesis EFO source, since it already uses the AWS SDK v2 async clients. Instead I would prefer to fix it in the test, either by setting a separate ELG on the test clients, or migrating to synchronous clients. I prefer the latter, since there is no need to spin up a thread pool for these tests!
The tradeoff would be that this issue could still surface for people running embedded Flink mini clusters. However, given the compromise I am willing to accept this.
@dmvk please give me your thoughts. Also, happy to discuss if easier.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1042982021
I think I see your point, as long as the SDK is loaded from the user jar, there are actually two instances of this SharedELG singleton, because they class have been loaded by a different classloader (this can be easily verified by running `jmap -histo <pid>`).
How does this change for the case when the SDK is not shipped with the user jar and is loaded from the system classpath (parent) instead?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1042994014
> However, since the AWS SDK jar is loaded by parent classloader I assume the Netty threads would be spawned via AWS SDK threads also loaded from parent classloader, thus attaching parent context classloader. But this is just a guess.
My intuition would be that this will use the user classloader as the threads are spawned from the user code. Maybe we could solve that by overriding the thread factory from the ELG and setting the context manually (not sure if thats doable)?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1039336785
It seems that there is still some pool, that we're not closing, can you take a look?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806683366
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
Right, I see. I elected to use the default as that's what the legacy `flink-connector-kinesis` (based on KPL) was using. Your thoughts, @dannycranmer ?
##########
File path: flink-connectors/flink-connector-aws-kinesis-firehose/src/test/java/org/apache/flink/connector/firehose/sink/KinesisFirehoseSinkITCase.java
##########
@@ -130,7 +128,7 @@ public void test() throws Exception {
List<S3Object> objects =
listBucketObjects(
- createS3Client(mockFirehoseContainer.getEndpoint(), httpClient),
+ s3AsyncClient,
Review comment:
(and this of course)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1039093666
@dmvk Dear David, please let us know if this fix encompasses all that you were hoping for after our discussion in #18553 . Looking forward!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292)
* 92739b14611f0fade361be4eb684fb9b85cf70aa UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1039336785
It seems that there is still some pool, that we're not closing, can you take a look?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806704643
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
The legacy connector based on KPL is not using async AWS SDK v2 client. Current (non FLIP-27) source is using async clients for EFO. This source creates [one client per sub task too](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java#L481), so this change will result in a larger number of threads being created and could cause a regression.
Since the Kinesis EFO source is currently using `2 x num cpus` I agree that `N x 2 x num cpus` is big change.
> It feels that some fairly low number should do
I agree. How about we pick 2 threads? This would follow the formula to 1 parallelism per CPU.
The other option is to try to calculate it dynamically to scale above 2 for bigger system, for instance:
```
thread_pool_size = max(2, (num_cpus / sink_parallelism) * 2)
# 2 CPUs at 8 parallelism (under provisioned)
thread_pool_size = max(2, (2 / 8) * 2) = 2
# 4 CPUs at 4 parallelism (target provisioning)
thread_pool_size = max(2, (4 / 4) * 2) = 2
# 32 CPUs at 4 parallelism (over provisioned)
thread_pool_size = max(2, (32 / 4) * 2) = 16
```
Maximum thread pool size here would be `2 x num cpus`. But does not scale for apps with many sinks, or clusters with many jobs.
> we should allow overrides from the user
I agree
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1047100687
> The reason was that the ELG issue was not well understood and we did not want to introduce additional risk. Now the ELG issue is well understood, I want to unblock other PRs so we can move forward.
👍
> Do you think this edge case is worth the investment now? Since it is non-standard to bundle and load these dependencies from the parent classloader.
As long as this is documented / user knows what needs to be done when this issue occurs, I'd say it's ok-ish limitation.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036363350
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 a118cb8ab6280c8ccbf308b2c985dc6c59a9016f (Fri Feb 11 16:01:07 UTC 2022)
**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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r807047407
##########
File path: flink-connectors/flink-connector-aws-kinesis-firehose/src/test/java/org/apache/flink/connector/firehose/sink/KinesisFirehoseSinkITCase.java
##########
@@ -130,7 +128,7 @@ public void test() throws Exception {
List<S3Object> objects =
listBucketObjects(
- createS3Client(mockFirehoseContainer.getEndpoint(), httpClient),
+ s3AsyncClient,
Review comment:
(and this of course)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
* b97b1992490a8c956075c0fd55661e4bf5de919d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
* b97b1992490a8c956075c0fd55661e4bf5de919d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1047100687
> The reason was that the ELG issue was not well understood and we did not want to introduce additional risk. Now the ELG issue is well understood, I want to unblock other PRs so we can move forward.
👍
> Do you think this edge case is worth the investment now? Since it is non-standard to bundle and load these dependencies from the parent classloader.
I think as long as this is documented / user knows what needs to be done when this issue occurs, I'd say it's ok-ish limitation.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r804879289
##########
File path: flink-connectors/flink-connector-aws-base/src/test/java/org/apache/flink/connector/aws/util/SdkEventLoopGroupBuilder.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * builder. work for additional i.(nformation regarding copyright owwnership.
+ * The ASF licenses builder. file to You under the Apache Licecopyrightownersh(nse, Version 2.0
+ * (the "License"); you may not use builder. file except in compLicense(liance wiVersion th
+ * 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.connector.aws.util;
+
+import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup;
+
+import java.util.concurrent.ThreadFactory;
+
+class SdkEventLoopGroupBuilder implements SdkEventLoopGroup.Builder {
Review comment:
nit: Seeing this in the test code it was not obvious this is a test proxy. Maybe a better name would be `SdkEventLoopGroupBuilderTestProxy`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1040494925
Thanks @dmvk , looks like the `.close()` is non-blocking from:
https://github.com/aws/aws-sdk-java-v2/blob/750f16463f4fe5d36139b410189cabba6d5ee5aa/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java#L198
I can also confirm this by adding a `Thread.sleep(100)` straight before the `System.out.println(X);` which removes any sdk/netty related threads from the thread dump.
Please let me know if this alleviates your concerns.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806683366
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
Right, I see. I elected to use the default as that's what the legacy `flink-connector-kinesis` (based on KPL) was using. Your thoughts, @dannycranmer ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292)
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
* b97b1992490a8c956075c0fd55661e4bf5de919d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1044720135
We have been blocking a series of other fixes behind this ELG issue. The reason was that the ELG issue was not well known and we did not want to introduce additional risk. Now the ELG issue is well understood, I want to unblock other PRs so we can move forward. We will proceed as follows:
- [FLINK-26064] Fix and re-enable the flaky test using synchronous clients
This will unblock the following:
- [FLINK-25948] Close KDS clients
- [FLINK-25848] Fail fast when invalid credentials supplied
- [FLINK-25846] Graceful shutdown on Interrupt
- [FLINK-25793] Reduce throttling
I have created a new issue to cover the ELG fix, and still marked as a blocker for 1.15:
- [FLINK-26256] ELG fix
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1040593983
I have been taking a deeper dive into this issue and am no longer inclined to make this change. I have verified that when running on a cluster, each job creates it's own `aws-java-sdk-NettyEventLoop` threads, and jobs do not interfere with each other. Running test cluster with 1 TM 1 CPU and 2 slots, posting 2 identical jobs. First job produces 2 threads:
```
"aws-java-sdk-NettyEventLoop-0-0" daemon prio=5 Id=76 RUNNABLE
"aws-java-sdk-NettyEventLoop-0-1" daemon prio=5 Id=77 RUNNABLE
```
Second job produces 2 threads:
```
"aws-java-sdk-NettyEventLoop-0-0" daemon prio=5 Id=95 RUNNABLE
"aws-java-sdk-NettyEventLoop-0-1" daemon prio=5 Id=96 RUNNABLE
```
Third job produces more threads etc. Killing jobs does not impact the other running jobs. You will notice the thread names are the same. This is because they are isolated in different job classloaders and cannot see each other to [increment the pool number](https://github.com/aws/aws-sdk-java-v2/blob/master/utils/src/main/java/software/amazon/awssdk/utils/ThreadFactoryBuilder.java#L75).
I have also managed to consistently reproduce this issue locally, and backup @dmvk findings. To reproduce this I made the following changes ([My test code](https://github.com/dannycranmer/flink/commit/6e42d2a052ca1ef2d13679159de9f9ad617121ac)):
- Switch the Flink job submission to `executeAsync()`
- Move the test client initialisation to after job submission
The problem in the flaky tests is that the `FlinkUserClassLoader` has the `Launcher$AppClassLoader` as it's parent classloader. This is the same classloader used by the test code. This means that both test code and Flink job can "see" the same ELG. As @dmvk pointed out, the Netty thread inherits current thread context classloader. The issue seems be be a race condition, since these clients are async, I guess that whoever makes the request first creates the Netty thread (not yet confirmed). The test blocks for Flink job to finish, which destroys the `FlinkUserClassLoader` and results in the classloader leak on the Netty threads.
Based on this, I am not inclined to create a new ELG per sub task, since this will potentially create a large resource overhead. This would also impact our current Kinesis EFO source, since it already uses the AWS SDK v2 async clients. Instead I would prefer to fix it in the test, either by setting a separate ELG on the test clients, or migrating to synchronous clients. I prefer the latter, since there is no need to spin up a thread pool for these tests!
The tradeoff would be that this issue could still surface for people running embedded Flink mini clusters. However, given the compromise I am willing to accept this.
@dmvk please give me your thoughts. Also, happy to discuss if easier.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806704643
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
The legacy connector based on KPL is not using async AWS SDK v2 client. Current (non FLIP-27) source is using async clients for EFO. This source creates [one client per sub task too](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java#L481), so this change will result in a larger number of threads being created and could cause a regression.
Since the Kinesis EFO source is currently using `2 x num cpus` I agree that `N x 2 x num cpus` is big change.
> It feels that some fairly low number should do
I agree. How about we pick 2 threads? This would follow the formula to 1 parallelism per CPU.
The other option is to try to calculate it dynamically to scale above 2 for bigger system, for instance:
```
thread_pool_size = max(2, (num_cpus / sink_parallelism) * 2)
# 2 CPUs at 8 parallelism (under provisioned)
thread_pool_size = max(2, (2 / 8) * 2) = 2
# 4 CPUs at 4 parallelism (target provisioning)
thread_pool_size = max(2, (4 / 4) * 2) = 2
# 32 CPUs at 4 parallelism (over provisioned)
thread_pool_size = max(2, (32 / 4) * 2) = 16
```
Maximum thread pool size here would be `2 x num cpus`. But does not scale for apps with many sinks, or clusters with many jobs.
> we should allow overrides from the user
I agree
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
The legacy connector based on KPL is not using async AWS SDK v2 client. Current (non FLIP-27) source is using async clients for EFO. This source creates [one client per sub task too](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java#L481), so this change will result in a larger number of threads being created and could cause a regression.
Since the Kinesis EFO source is currently using `2 x num cpus` (assuming it is using the shared group) I agree that `N x 2 x num cpus` is big change.
> It feels that some fairly low number should do
I agree. How about we pick 2 threads? This would follow the formula to 1 parallelism per CPU.
The other option is to try to calculate it dynamically to scale above 2 for bigger system, for instance:
```
thread_pool_size = max(2, (num_cpus / sink_parallelism) * 2)
# 2 CPUs at 8 parallelism (under provisioned)
thread_pool_size = max(2, (2 / 8) * 2) = 2
# 4 CPUs at 4 parallelism (target provisioning)
thread_pool_size = max(2, (4 / 4) * 2) = 2
# 32 CPUs at 4 parallelism (over provisioned)
thread_pool_size = max(2, (32 / 4) * 2) = 16
```
Maximum thread pool size here would be `2 x num cpus`. But does not scale for apps with many sinks, or clusters with many jobs.
> we should allow overrides from the user
I agree
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
The legacy connector based on KPL is not using async AWS SDK v2 client. Current (non FLIP-27) source is using async clients for EFO. This source creates [one client per sub task too](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java#L481), so this change will result in a larger number of threads being created and could cause a regression.
Since the Kinesis EFO source is currently using `2 x num cpus` (assuming it is using the shared group) I agree that `N x 2 x num cpus` is big change.
> It feels that some fairly low number should do
I agree. How about we pick 2 threads? This would follow the formula when we have 1 parallelism per CPU.
The other option is to try to calculate it dynamically to scale above 2 for bigger system, for instance:
```
thread_pool_size = max(2, (num_cpus / sink_parallelism) * 2)
# 2 CPUs at 8 parallelism (under provisioned)
thread_pool_size = max(2, (2 / 8) * 2) = 2
# 4 CPUs at 4 parallelism (target provisioning)
thread_pool_size = max(2, (4 / 4) * 2) = 2
# 32 CPUs at 4 parallelism (over provisioned)
thread_pool_size = max(2, (32 / 4) * 2) = 16
```
Maximum thread pool size here would be `2 x num cpus`. But does not scale for apps with many sinks, or clusters with many jobs.
> we should allow overrides from the user
I agree
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806704643
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
The legacy connector based on KPL is not using async AWS SDK v2 client. Current (non FLIP-27) source is using async clients for EFO. This source creates [one client per sub task too](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java#L481), so this change will result in a larger number of threads being created and could cause a regression.
Since the Kinesis EFO source is currently using `2 x num cpus` (assuming it is using the shared group) I agree that `N x 2 x num cpus` is big change.
> It feels that some fairly low number should do
I agree. How about we pick 2 threads? This would follow the formula to 1 parallelism per CPU.
The other option is to try to calculate it dynamically to scale above 2 for bigger system, for instance:
```
thread_pool_size = max(2, (num_cpus / sink_parallelism) * 2)
# 2 CPUs at 8 parallelism (under provisioned)
thread_pool_size = max(2, (2 / 8) * 2) = 2
# 4 CPUs at 4 parallelism (target provisioning)
thread_pool_size = max(2, (4 / 4) * 2) = 2
# 32 CPUs at 4 parallelism (over provisioned)
thread_pool_size = max(2, (32 / 4) * 2) = 16
```
Maximum thread pool size here would be `2 x num cpus`. But does not scale for apps with many sinks, or clusters with many jobs.
> we should allow overrides from the user
I agree
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31571",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
* b97b1992490a8c956075c0fd55661e4bf5de919d Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31571)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1040494925
Thanks @dmvk , looks like the `.close()` is non-blocking from:
https://github.com/aws/aws-sdk-java-v2/blob/750f16463f4fe5d36139b410189cabba6d5ee5aa/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java#L198
I can also confirm this by adding a `Thread.sleep(100)` straight before the `System.out.println(X);` which removes any sdk/netty related threads from the thread dump.
Please let me know if this alleviates your concerns.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r804880817
##########
File path: flink-connectors/flink-connector-aws-base/src/test/java/org/apache/flink/connector/aws/util/SdkEventLoopGroupBuilder.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * builder. work for additional i.(nformation regarding copyright owwnership.
+ * The ASF licenses builder. file to You under the Apache Licecopyrightownersh(nse, Version 2.0
+ * (the "License"); you may not use builder. file except in compLicense(liance wiVersion th
+ * 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.connector.aws.util;
+
+import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup;
+
+import java.util.concurrent.ThreadFactory;
+
+class SdkEventLoopGroupBuilder implements SdkEventLoopGroup.Builder {
Review comment:
Got it makes sense, will change.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1041882765
@dmvk Would be good to get your thoughts please.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1042989104
> If there are two jobs running on the same TM and they're both using the shared ELG, how comes they can't interfere with each other?
When 2 jobs are running on the same TM, they are using separate ELG already, because they are created in static namespace of Flink User classloader.
> In general I'm inclined to use the shared reference counted ELG, but we'd probably have to scope it per jobId.
Based on above, we already have a shared reference counting ELG scoped per job. So for a single job, multiple sources/sinks using AWS async clients will share the same thread pool
> I think I see your point, as long as the SDK is loaded from the user jar, there are actually two instances of this SharedELG singleton, because they class have been loaded by a different classloader (this can be easily verified by running jmap -histo <pid>).
Correct yes. I confirmed this by observing the new Netty threads created for each ELG, as explained in my previous comment.
> How does this change for the case when the SDK is not shipped with the user jar and is loaded from the system classpath (parent) instead?
In this case we would have a single shared ELG for all jobs on the TM. However, since the AWS SDK jar is loaded by parent classloader I assume the Netty threads would be spawned via AWS SDK classes also loaded from parent classloader, thus attaching parent context classloader. But this is just a guess.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1044733518
> Based on above, we already have a shared reference counting ELG scoped per job. So for a single job, multiple sources/sinks using AWS async clients will share the same thread pool
FYI Just occurred to me that current Kinesis EFO source shades and relocates AWS SDK/netty classes. So for Kinesis EFO source and Kinesis Async Sink we would actually have 2x ELG, caused by the shading.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r804883389
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
> SdkEventLoopGroup.builder()
If anyone is interested what the default behaviour is, because I was, then here is a link:
- https://github.com/aws/aws-sdk-java-v2/blob/master/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java#L148
It will use `SharedSdkEventLoopGroup::get`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806060325
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
I think this should be lowered. This will basically mean that for N tasks (running in the same process), we'll have `N x 2 x num cpus` threads. It feels that some fairly low number should do for most use cases and we should allow overrides from the user.
WDYT?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806704643
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
The legacy connector based on KPL is not using async AWS SDK v2 client. Current (non FLIP-27) source is using async clients for EFO. This source creates [one client per sub task too](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java#L481), so this change will result in a larger number of threads being created and could cause a regression.
Since the Kinesis EFO source is currently using `2 x num cpus` (assuming it is using the shared group) I agree that `N x 2 x num cpus` is big change.
> It feels that some fairly low number should do
I agree. How about we pick 2 threads? This would follow the formula when we have 1 parallelism per CPU.
The other option is to try to calculate it dynamically to scale above 2 for bigger system, for instance:
```
thread_pool_size = max(2, (num_cpus / sink_parallelism) * 2)
# 2 CPUs at 8 parallelism (under provisioned)
thread_pool_size = max(2, (2 / 8) * 2) = 2
# 4 CPUs at 4 parallelism (target provisioning)
thread_pool_size = max(2, (4 / 4) * 2) = 2
# 32 CPUs at 4 parallelism (over provisioned)
thread_pool_size = max(2, (32 / 4) * 2) = 16
```
Maximum thread pool size here would be `2 x num cpus`. But does not scale for apps with many sinks, or clusters with many jobs.
> we should allow overrides from the user
I agree
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
* b97b1992490a8c956075c0fd55661e4bf5de919d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1042989104
> If there are two jobs running on the same TM and they're both using the shared ELG, how comes they can't interfere with each other?
When 2 jobs are running on the same TM, they are using separate ELG already, because they are created in static namespace of Flink User classloader.
> In general I'm inclined to use the shared reference counted ELG, but we'd probably have to scope it per jobId.
Based on above, we already have a shared reference counting ELG scoped per job. So for a single job, multiple sources/sinks using AWS async clients will share the same thread pool
> I think I see your point, as long as the SDK is loaded from the user jar, there are actually two instances of this SharedELG singleton, because they class have been loaded by a different classloader (this can be easily verified by running jmap -histo <pid>).
Correct yes. I confirmed this by observing the new Netty threads created for each ELG, as explained in my previous comment.
> How does this change for the case when the SDK is not shipped with the user jar and is loaded from the system classpath (parent) instead?
In this case we would have a single shared ELG for all jobs on the TM. However, since the AWS SDK jar is loaded by parent classloader I assume the Netty threads would be spawned via AWS SDK threads also loaded from parent classloader, thus attaching parent context classloader. But this is just a guess.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1042989104
> If there are two jobs running on the same TM and they're both using the shared ELG, how comes they can't interfere with each other?
When 2 jobs are running on the same TM, they are already using separate ELG already, because they are created in static namespace of Flink User classloader.
> In general I'm inclined to use the shared reference counted ELG, but we'd probably have to scope it per jobId.
Based on above, we already have a shared reference counting ELG scoped per job. So for a single job, multiple sources/sinks using AWS async clients will share the same thread pool
> I think I see your point, as long as the SDK is loaded from the user jar, there are actually two instances of this SharedELG singleton, because they class have been loaded by a different classloader (this can be easily verified by running jmap -histo <pid>).
Correct yes. I confirmed this by observing the new Netty threads created for each ELG, as explained in my previous comment.
> How does this change for the case when the SDK is not shipped with the user jar and is loaded from the system classpath (parent) instead?
In this case we would have a single shared ELG for all jobs on the TM. However, since the AWS SDK jar is loaded by parent classloader I assume the Netty threads would be spawned via AWS SDK threads also loaded from parent classloader, thus attaching parent context classloader. But this is just a guess.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1043014193
> Maybe we could solve that by overriding the thread factory from the ELG and setting the context manually (not sure if thats doable)?
Possibly. Would we check which classloader the ELG is using and replicate that to the thread context?
If we are going to this extent, maybe we could add a `static Map<JobId, ELG>` in the AWS util class and to create a separate ELG per job as discussed (and copy the counting aspects of `SharedSdkEventLoopGroup`). I am not sure how we would acquire the job ID though, are there any context objects we can grab this from?
Do you think this edge case is worth the investment now? Since it is non-standard to bundle and load these dependencies from the parent classloader. I am sure there are plenty of edge cases similar to this where static namespace in parent loader cause issues. For example, Kafka Security providers.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1044720135
We have been blocking a series of other fixes behind this ELG issue. The reason was that the ELG issue was not well understood and we did not want to introduce additional risk. Now the ELG issue is well understood, I want to unblock other PRs so we can move forward. We will proceed as follows:
- [FLINK-26064] Fix and re-enable the flaky test using synchronous clients
This will unblock the following:
- [FLINK-25948] Close KDS clients
- [FLINK-25848] Fail fast when invalid credentials supplied
- [FLINK-25846] Graceful shutdown on Interrupt
- [FLINK-25793] Reduce throttling
I have created a new issue to cover the ELG fix, and still marked as a blocker for 1.15:
- [FLINK-26256] ELG fix
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] CrynetLogistics commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r804789573
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -274,6 +275,7 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
httpClientBuilder
+ .eventLoopGroupBuilder(SdkEventLoopGroup.builder())
Review comment:
Looks like the httpClient closes the ELG when it itself is closed: https://github.com/aws/aws-sdk-java-v2/blob/master/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java#:~:text=public%20void%20close()%20%7B
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a118cb8ab6280c8ccbf308b2c985dc6c59a9016f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282)
* 6bbcfb33a684bb89a0873eab4f5c2799e0b506fe Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288)
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b16c92d2ca02afc1c52fc3c0383ccc55c7485991 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dannycranmer commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dannycranmer commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r805832727
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
Please confirm that defaults are actually used here. I followed the code through and arrived here, when using a DefaultBuilder without setting any values. Seems to default to 0 threads:
- https://github.com/aws/aws-sdk-java-v2/blob/52a737b464d6d91247f1c53cfc994eeaeb375b5f/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/SdkEventLoopGroup.java#L127
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
Please confirm which defaults are actually used here. I followed the code through and arrived here, when using a DefaultBuilder without setting any values. Seems to default to 0 threads:
- https://github.com/aws/aws-sdk-java-v2/blob/52a737b464d6d91247f1c53cfc994eeaeb375b5f/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/SdkEventLoopGroup.java#L127
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* 92739b14611f0fade361be4eb684fb9b85cf70aa Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562)
* b97b1992490a8c956075c0fd55661e4bf5de919d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on a change in pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
dmvk commented on a change in pull request #18733:
URL: https://github.com/apache/flink/pull/18733#discussion_r806060325
##########
File path: flink-connectors/flink-connector-aws-base/src/main/java/org/apache/flink/connector/aws/util/AWSGeneralUtil.java
##########
@@ -273,7 +274,15 @@ public static SdkAsyncHttpClient createAsyncHttpClient(
public static SdkAsyncHttpClient createAsyncHttpClient(
final AttributeMap config, final NettyNioAsyncHttpClient.Builder httpClientBuilder) {
+ return createAsyncHttpClient(config, httpClientBuilder, SdkEventLoopGroup.builder());
Review comment:
I think this should be lowered. This will basically mean that for N tasks (running in the same process), we'll have `N x 2 x num cpus` threads. It feels that some fairly low number should do for most use cases and we should allow overrides from the user.
WDYT?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #18733: [FLINK-26064][connectors/firehose,kinesis] Using own event loop group…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #18733:
URL: https://github.com/apache/flink/pull/18733#issuecomment-1036367128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31282",
"triggerID" : "a118cb8ab6280c8ccbf308b2c985dc6c59a9016f",
"triggerType" : "PUSH"
}, {
"hash" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31288",
"triggerID" : "6bbcfb33a684bb89a0873eab4f5c2799e0b506fe",
"triggerType" : "PUSH"
}, {
"hash" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dacff5752d32e487590ae582e94a7e740c1df2d7",
"triggerType" : "PUSH"
}, {
"hash" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31292",
"triggerID" : "b16c92d2ca02afc1c52fc3c0383ccc55c7485991",
"triggerType" : "PUSH"
}, {
"hash" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31562",
"triggerID" : "92739b14611f0fade361be4eb684fb9b85cf70aa",
"triggerType" : "PUSH"
}, {
"hash" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31571",
"triggerID" : "b97b1992490a8c956075c0fd55661e4bf5de919d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dacff5752d32e487590ae582e94a7e740c1df2d7 UNKNOWN
* b97b1992490a8c956075c0fd55661e4bf5de919d Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=31571)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org