ref(server): Introduce a queue to the concurrency limiter#562
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
==========================================
+ Coverage 87.40% 87.53% +0.13%
==========================================
Files 93 93
Lines 14787 14969 +182
==========================================
+ Hits 12924 13103 +179
- Misses 1863 1866 +3
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
With max=0 and a non-zero queue, callers were admitted into the queue and waited the full timeout for a task permit that could never exist. Guard at the top of `acquire` so the at-capacity mode stays instant.
Prevents u32 wrapping when max + queue overflows in release builds.
Replace positional (u32, u32) arguments with a #[non_exhaustive] Stats struct so new fields can be added without breaking callers.
Replace the manual elapsed/match block with a timer guard around acquire and a single inspect_err for rejection logging.
Replace with_concurrency_limit + with_concurrency_queue with a single with_concurrency that takes a pre-configured ConcurrencyLimiter. The limiter module is now pub so callers can construct it directly.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f7acaac. Configure here.
| Ok(ConcurrencyPermit { | ||
| permit: Some(permit), | ||
| task_permit: Some(task_permit), | ||
| queue_permit: Some(queue_permit), |
There was a problem hiding this comment.
Spurious rejects during bulk acquire
Medium Severity
try_acquire_many takes outer queue permits before the inner execution permits. When the inner acquire fails, those queue slots are held until the function returns. On a multi-threaded runtime, a concurrent acquire can observe a full outer semaphore and get AtCapacity even though queue capacity remains and the bulk reservation is about to give up its slots.
Reviewed by Cursor Bugbot for commit f7acaac. Configure here.
There was a problem hiding this comment.
That's an inherent limit of the design. We will solve this in a follow-up by removing acquire_many and changing batch-processing so it polls individually.
| /// # Default | ||
| /// | ||
| /// `0` | ||
| pub concurrency_queue: u32, |
There was a problem hiding this comment.
| pub concurrency_queue: u32, | |
| pub concurrency_queue_size: u32, |
Maybe?
There was a problem hiding this comment.
I'm also not entirely happy with this. Was also thinking of max_queued. Let's wait for the follow-up that introduces another parameter for batch requests and then reassess.
2 participants
The concurrency limiter can now absorb brief capacity bursts by queuing requests instead of rejecting them immediately. This is implemented through semaphores: an outer semaphore (capacity
max + queue) gates total participants, and an inner semaphore (capacitymax) gates execution. Requests that pass the outer gate wait up to a configurable timeout for an execution permit; requests that exceed both limits are rejected withAtCapacityas before.With the default
concurrency_queue = 0, both semaphores have the same capacity and behavior is identical to the current immediate-reject model. In this case, there is no queuing and no added latency.Two new config fields control the queue. Sizing guidance: set the queue depth to roughly
permit_release_rate × acceptable_added_latency, and keep the timeout comfortably below client request timeouts.Batch
stream()reservations continue to use non-blockingtry_acquire_manyand do not enter the queue.Ref FS-447