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

Improve reliability of TmpDb startup #655

Merged
merged 3 commits into from
Jul 1, 2024
Merged

Improve reliability of TmpDb startup #655

merged 3 commits into from
Jul 1, 2024

Conversation

jbearer
Copy link
Member

@jbearer jbearer commented Jul 1, 2024

Because we were only checking for database readiness from inside the container, there was a case where we would consider the TmpDb "ready" and allow connections before the Docker daemon had actually exposed the database port to the host. This change explicitly checks that the port is exposed on the host before returning, with no extra dependencies on the host side.

This fixes an intermittent "Connection Refused" error that @rob-maron and I were seeing while debugging an unrelated sequencer test failure.

Because we were only checking for database readiness from inside
the container, there was a case where we would consider the TmpDb
"ready" and allow connections before the Docker daemon had actually
exposed the database port to the host. This change explicitly checks
that the port is exposed on the host before returning, with no extra
dependencies on the host side.
@jbearer jbearer requested review from babdor and rob-maron July 1, 2024 14:08
@jbearer jbearer self-assigned this Jul 1, 2024
src/data_source/storage/sql.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@tbro tbro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only thought is that 1 second seems like a long time for this sort of thing to occur, so might be worth reducing the wait time.

jbearer added 2 commits July 1, 2024 10:24
The timeout defaults to a rather conservative 1 minute, but is
adjustable via the env var `SQL_TMP_DB_CONNECT_TIMEOUT`, which is
a number of seconds.
@rob-maron rob-maron self-requested a review July 1, 2024 14:36
@jbearer jbearer merged commit a44ad35 into main Jul 1, 2024
8 checks passed
@jbearer jbearer deleted the jb/tmp-db-startup branch July 1, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants