Make queue claimer cap configurable#214
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a cached ChangesConfigurable Max Claimers Per Queue
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 56 minutes and 52 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
awa-worker/src/dispatcher.rs (1)
588-588: ⚡ Quick winCache the result of
max_claimers_per_queue()— avoid a global env-lock on every poll.
max_claimers_per_queue()callsstd::env::var, which acquires the process-wide environment mutex on every invocation. Under load,drain_readyloops callingpoll_oncecontinuously (once per claimed batch), so this mutex acquisition + string parse fires on every DB round-trip. The value is logically static for the process lifetime; there is no benefit to re-reading it each time.The cleanest fix is a
std::sync::OnceLock:♻️ Proposed refactor using `OnceLock`
+use std::sync::OnceLock; + +static MAX_CLAIMERS: OnceLock<i16> = OnceLock::new(); + fn max_claimers_per_queue() -> i16 { - std::env::var("AWA_MAX_CLAIMERS_PER_QUEUE") - .ok() - .and_then(|value| value.parse::<i16>().ok()) - .filter(|value| *value > 0) - .unwrap_or(MAX_CLAIMERS_PER_QUEUE) + *MAX_CLAIMERS.get_or_init(|| { + // ... same parsing + warn! logic as above ... + std::env::var("AWA_MAX_CLAIMERS_PER_QUEUE") + .ok() + .and_then(|v| v.parse::<i16>().ok()) + .filter(|&v| v > 0) + .unwrap_or(MAX_CLAIMERS_PER_QUEUE) + }) }Alternatively, read and store the value in
Dispatcher::new/Dispatcher::with_concurrencyand add it as a field — which also makes it straightforward to test with different values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@awa-worker/src/dispatcher.rs` at line 588, The call to max_claimers_per_queue() is performed inside the hot loop (e.g., drain_ready -> poll_once) and repeatedly invokes std::env::var, causing a global env mutex contention; cache this value instead by initializing it once and reusing it: either add a field to Dispatcher (set in Dispatcher::new or Dispatcher::with_concurrency) to store the parsed max_claimers_per_queue value, or use a process-wide std::sync::OnceLock to compute and store the value on first access, then replace direct calls to max_claimers_per_queue() in the polling path with the cached value; ensure the cached type matches the current return type and adjust tests to set the value via the chosen initialization path if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@awa-worker/src/dispatcher.rs`:
- Around line 21-27: The function max_claimers_per_queue currently silently
falls back to MAX_CLAIMERS_PER_QUEUE on parse errors or non-positive values;
update it to detect when AWA_MAX_CLAIMERS_PER_QUEUE is set but invalid or <= 0
and emit a warning (e.g., using log::warn!) that includes the raw env value and
parse error or the fact it was non-positive, then return the existing fallback;
modify the code in max_claimers_per_queue to inspect
std::env::var("AWA_MAX_CLAIMERS_PER_QUEUE") result, attempt parse with map_err
to capture parse errors, log appropriately when Err or when parsed_value <= 0,
and only use unwrap_or(MAX_CLAIMERS_PER_QUEUE) after logging.
---
Nitpick comments:
In `@awa-worker/src/dispatcher.rs`:
- Line 588: The call to max_claimers_per_queue() is performed inside the hot
loop (e.g., drain_ready -> poll_once) and repeatedly invokes std::env::var,
causing a global env mutex contention; cache this value instead by initializing
it once and reusing it: either add a field to Dispatcher (set in Dispatcher::new
or Dispatcher::with_concurrency) to store the parsed max_claimers_per_queue
value, or use a process-wide std::sync::OnceLock to compute and store the value
on first access, then replace direct calls to max_claimers_per_queue() in the
polling path with the cached value; ensure the cached type matches the current
return type and adjust tests to set the value via the chosen initialization path
if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f38fd39-a4f3-46d6-bf7d-82c6ffc9c24b
📒 Files selected for processing (1)
awa-worker/src/dispatcher.rs
hardbyte
force-pushed
the
brian/dispatcher-claimer-cap-env
branch
from
312d217 to
442bc60
Compare
hardbyte
force-pushed
the
brian/dispatcher-claimer-cap-env
branch
from
442bc60 to
91b144b
Compare
1 participant
Summary
Validation
Summary by CodeRabbit