test(gateway): de-flake concurrent-compression lock test with a barrier - #41971
Merged
Conversation
test_concurrent_compressions_same_session_serialize relied on a time.sleep(0.25) inside the stubbed compressor to make the two threads overlap inside the per-session lock window. Under CI CPU starvation that sleep is insufficient: one thread can acquire -> compress -> rotate -> RELEASE the lock before the other reaches try_acquire, so both acquire on the shared session_id and both compress (the recurring 'Expected exactly one agent to compress, got 2' failure on shard test (1)). Replace the timing dependency with a threading.Barrier(2) wrapped around the shared db's try_acquire_compression_lock: both threads rendezvous immediately before the real (atomic) acquire, guaranteeing genuine simultaneous contention regardless of scheduling. The real lock logic is unchanged and still picks exactly one winner — this only fixes the test's overlap guarantee. Restored after join so the post-join lock-leak assertion hits the unwrapped method. Verified: 20/20 plain + 15/15 under all-core CPU stress (load avg ~4.6), where the old version flaked.
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-assignment |
2 |
First entries
tests/gateway/test_compression_concurrent_sessions.py:192: [invalid-assignment] invalid-assignment: Object of type `bound method SessionDB.try_acquire_compression_lock(session_id: str, holder: str, ttl_seconds: int | float = ...) -> bool` is not assignable to attribute `try_acquire_compression_lock` of type `def try_acquire_compression_lock(self, session_id: str, holder: str, ttl_seconds: int | float = ...) -> bool`
tests/gateway/test_compression_concurrent_sessions.py:171: [invalid-assignment] invalid-assignment: Object of type `def _barriered_acquire(...) -> Unknown` is not assignable to attribute `try_acquire_compression_lock` of type `def try_acquire_compression_lock(self, session_id: str, holder: str, ttl_seconds: int | float = ...) -> bool`
✅ Fixed issues: none
Unchanged: 5425 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tests/gateway/test_compression_concurrent_sessions.py::test_concurrent_compressions_same_session_serializeno longer flakes under CI load. It was failing on shardtest (1)with "Expected exactly one agent to compress, got 2" — including onmainHEAD, independent of any feature PR.Root cause (test-only, not a lock bug): the test relied on a
time.sleep(0.25)inside the stubbed compressor to make the two threads overlap inside the per-session lock window. Under CI CPU starvation that sleep is insufficient — one thread can acquire → compress → rotate → release the lock before the other thread even reachestry_acquire. Both then acquire on the sharedsession_idand both compress. The real per-session lock (#34351) is correct; the test's overlap was a timing accident.Changes
tests/gateway/test_compression_concurrent_sessions.py: wrap the shared db'stry_acquire_compression_lockin athreading.Barrier(2)so both threads rendezvous immediately before the real atomic acquire — guaranteeing genuine simultaneous contention regardless of scheduling. The real lock logic is untouched; it still picks exactly one winner. The wrapper is restored afterjoin()so the post-join lock-leak assertion hits the unwrapped method.Validation
CPU-stress reproduction is the key check: the old version's failure mode (one thread finishing before the other starts) is now structurally impossible.
Infographic