fix(ci): terminate db sessions before DROP DATABASE in hogli#60608
Conversation
DROP DATABASE fails with 'database posthog is being accessed by other users' when idle connections remain from earlier CI steps. Add pg_terminate_backend() call before DROP to clear lingering sessions. This mirrors the existing pattern in posthog/ducklake/common.py.
|
@mendral-app if this affects certain tests, shouldn't we widen the path filters to include them here? also fix test_restore_schema_dump_recreate_cleans_up_after_failure |
|
Reviews (1): Last reviewed commit: "fix(ci): terminate db sessions before DR..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 257a1bdb90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"SELECT pg_terminate_backend(pid) FROM pg_stat_activity " | ||
| f"WHERE datname = '{target_db}' AND pid <> pg_backend_pid();" |
There was a problem hiding this comment.
When restore_schema_dump(..., recreate=True) fails after starting the restore, the exception handler calls _recreate_database() a second time, so this new admin statement is now emitted twice. The existing tools/hogli-commands/hogli_commands/tests/test_db_schema.py::test_restore_schema_dump_recreate_cleans_up_after_failure still asserts the exact admin SQL sequence is only DROP/CREATE twice, so the hogli command test suite will fail until that expectation includes the termination statements.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Fixes flaky CI DROP DATABASE failures in hogli's _recreate_database by terminating other Postgres sessions on the target DB before dropping, mirroring the existing pattern in posthog/ducklake/common.py.
Changes:
- Add
SELECT pg_terminate_backend(...)call in_recreate_databasebeforeDROP DATABASE/CREATE DATABASE.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _psql_admin( | ||
| f"SELECT pg_terminate_backend(pid) FROM pg_stat_activity " | ||
| f"WHERE datname = '{target_db}' AND pid <> pg_backend_pid();" | ||
| ) |
|
Good catch on the test! I've pushed a fix in 341927a — Regarding path filters: the CI workflow ( CI is green (the only failure is an unrelated Semgrep Package Managers check). |
|
@mendral-app what I meant was you said "affects Integration Tests and MCP test setup." But adjusting this hogli command does not have a blast radius of actually testing that. Hence my suggestion to widen path filters. Assess, decide and open follow up PR. This one here has been merged. |
|
Good point — you're right that changes to I've opened a follow-up PR to widen the path filters: #60612 It adds |
Summary
DROP DATABASEfails withdatabase "posthog" is being accessed by other usersbecause idle PostgreSQL connections remain from earlier setup steps.pg_terminate_backend()call beforeDROP DATABASEin the hogli_recreate_databasehelper, matching the existing pattern inposthog/ducklake/common.py.Reported in multiple PRs today — affects Integration Tests and MCP test setup.
Note
Created by Mendral. Tag @mendral-app with feedback or questions.