Skip to content
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

Increase timeout in container test #11257

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

nineinchnick
Copy link
Member

Description

When #10413 added a healthcheck, it also replaced looping over trino-cli in the container test script with docker inspect. Since executing trino-cli could time out after the container started but before the server was ready, it actually added to the total timeout of the loop. Now, none of the commands in the loop are blocking for longer periods, so the timeout is easier to control.

We measured that the container can take up to 7 minutes to start on arm64 using emulation, so I suggest setting the timeout to 10 minutes.

Is this change a fix, improvement, new feature, refactoring, or other?
other, fix test flakiness

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)
other, test script

How would you describe this change to a non-technical end user or system administrator?
n/a

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@findepi
Copy link
Member

findepi commented Mar 1, 2022

When #10413 added a healthcheck, it also replaced looping over trino-cli in the container test script with docker inspect. Since executing trino-cli could time out after the container started but before the server was ready, it actually added to the total timeout of the loop. Now, none of the commands in the loop are blocking for longer periods, so the timeout is easier to control.

We measured that the container can take up to 7 minutes to start on arm64 using emulation, so I suggest setting the timeout to 10 minutes.

Add this to commit description please.

When the container healthcheck was added, it also replaced looping
over trino-cli in the container test script with docker inspect.
Since executing trino-cli could time out after the container started
but before the server was ready, it actually added to the total timeout
of the loop. Now, none of the commands in the loop are blocking for
longer periods, so the timeout is easier to control.

It was measured that the container can take up to 7 minutes to start on arm64
using emulation, so the timeout is raised to 10 minutes.
@nineinchnick nineinchnick force-pushed the container-test-timeouts branch from 8e7b087 to d57903b Compare March 2, 2022 08:18
@findepi
Copy link
Member

findepi commented Mar 2, 2022

Previous build https://github.com/trinodb/trino/actions/runs/1917207395 green except for #11275

@findepi findepi merged commit 3a64e70 into trinodb:master Mar 2, 2022
@nineinchnick nineinchnick deleted the container-test-timeouts branch March 2, 2022 09:51
@github-actions github-actions bot added this to the 372 milestone Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants