-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-26300][connectors/common] Increased timeout for LocalstackCont… #18887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@flinkbot azure run |
|
@flinkbot run azure |
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit b16fd24 (Tue Feb 22 16:57:27 UTC 2022) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe 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 Bot commandsThe @flinkbot bot supports the following commands:
|
| protected void waitUntilReady() { | ||
| try { | ||
| Thread.sleep(10000); | ||
| Thread.sleep(30_000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please justify the reason for this.
Is there any other way we can establish if the test stack is ready?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't see a good way to poll whether the container is ready. On the testcontainers side, I tried tracking the output of isRunning()/isCreated()/isReusable() through successful and unsuccessful runs and none of them are telling. Anything on the localstack api side puts it in a funny state in the same way the s3bucket read strategy does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a solution here: https://ignas.me/tech/waiting-localstack-s3-start/
I’ve noticed that hitting Refresh button in LocalStack Dashboard calls /graph endpoint to know if and how many nodes (mock services) are available, i.e. healthy and ready to accept incoming requests. Bullseye!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create a follow up for this improvement and we can merge this to unblock the testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have created this: https://issues.apache.org/jira/browse/FLINK-26309.
b16fd24 to
4c10f8e
Compare
|
@flinkbot run azure |
2 similar comments
|
@flinkbot run azure |
|
@flinkbot run azure |
…ainer to start
What is the purpose of the change
Fix the
LocalstackContainerstartup loop.Explanation: The localstack container is used to mock aws services so tests can hit a mock endpoint. When we start the container, it occasionally fails with the exception:
IOException - closed before protocol could be determined.This is what happens whenever we make a request (e.g. read from mock s3) prior to the container being ready**. A similar issue was resolved in Kinesalite by increasing the timeout from 1s to 10s, but Localstack is a larger box. So here we increase the wait to 30s.** This is a known limitation, please see issue #1202 in Localstack/localstack or this blog post.
Brief change log
Increased timeout of container startup to 30s from 10s.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no)nDocumentation