Repository navigation
Prove concurrent API reads with a barrier instead of timings - #435
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 38cd5e5b1b
ℹ️ 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".
| self._barrier = barrier | ||
|
|
||
| def execute(self, *args, **kwargs): | ||
| self._barrier.wait() |
There was a problem hiding this comment.
This rendezvous occurs before calling the real cursor's execute, so it only proves that four requests reached the proxy concurrently. If adapter.cursor() returns distinct cursor wrappers that serialize through a shared internal lock—as BaseDatabaseAdapter.cursor() does—all four calls pass this barrier, then execute one at a time; the identity assertion also passes because the wrappers are distinct. The test therefore accepts the exact shared-connection serialization regression it claims to detect.
Useful? React with 👍 / 👎.
test_concurrent_reads_do_not_serializeasserted that 4 concurrent requests finished within 2.5x a single request's wall time. On loaded CI runners that ratio regularly blew past the bound even though reads were concurrent (e.g. 843ms vs a 250ms budget), so it failed in most recent runs.The test now sends 4 concurrent
/rawqueries that call a DuckDB Python UDF which waits on a barrier until all 4 queries are executing inside DuckDB at once. Reads serialized anywhere (an app lock, a lock-wrapped cursor like the base adapter fallback, or one shared connection) can never all arrive, so the barrier times out and the test fails; concurrent reads pass regardless of runner speed.