Skip to content

fix: Database migration race condition in docker with Redis#6296

Open
Walkablenormal wants to merge 4 commits intokeephq:mainfrom
Walkablenormal:fix_6294_race_cond_database_migration
Open

fix: Database migration race condition in docker with Redis#6296
Walkablenormal wants to merge 4 commits intokeephq:mainfrom
Walkablenormal:fix_6294_race_cond_database_migration

Conversation

@Walkablenormal
Copy link
Copy Markdown
Contributor

Closes #6294

📑 Description

As stated in issue #6294, when deploying in Docker with Redis a race condition is triggered which only allows for a little over 5 seconds for database migration until the API workers start, and try to query the database. In this PR, database migration is moved up and out of the workers. Now it will run before the workers start. The migrate_db() function handles if the user has set SKIP_DB_CREATION to true.

✅ Checks

  • My pull request adheres to the code style of this project
  • [N] My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

Updated entrypoint.sh to run database migration and modified execution of the command.

Signed-off-by: Walkablenormal <rubenvankomen@gmail.com>
Removed database migration call from server startup. Called separately on entrypoint 

Signed-off-by: Walkablenormal <rubenvankomen@gmail.com>
Signed-off-by: Walkablenormal <rubenvankomen@gmail.com>
Signed-off-by: Walkablenormal <rubenvankomen@gmail.com>
@Walkablenormal Walkablenormal marked this pull request as ready for review April 25, 2026 19:04
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 25, 2026
@Walkablenormal
Copy link
Copy Markdown
Contributor Author

Walkablenormal commented Apr 25, 2026

Hi,

Even with the failing tests, this seems to be safe to merge. The E2E test and unit test failing seems to be unrelated to this PR.

I'll take a look if I can fix these tests in a separate PR.

EDIT: Fixed unit test in #6298, if you want, you can/may merge that first and check if this PR correctly runs the unit tests.

@shahargl Can you look at this and merge please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Race condition on database migration when deploying Keep with Redis

1 participant