fix(queue): make the effective store visible and surface delivery stats - #493
Conversation
The builtin adapter silently fell back to the in-memory store when the stored configuration lacked an adapter entry, even if the --config seed asked for file_based persistence. Boot now logs the effective store (file_based + path, or in_memory) and warns when the seed's adapter is discarded in favor of the authoritative stored value. engine::queue::topic_stats now returns the delivered/failed counters the store already tracks, so queue throughput is observable via the API.
ytallo
added
the
no-ticket
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 42 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThe queue worker now logs store initialization, exposes delivered and failed topic outcomes, and warns when a configuration seed’s adapter differs from the stored adapter. ChangesQueue runtime updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@queue/src/main.rs`:
- Around line 92-100: Update the adapter comparison in the seed validation block
to run only when seed.adapter is Some, while preserving the existing mismatch
warning and stored-configuration authority behavior for explicitly provided
adapters.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4fc3201-85e2-455a-9a42-fda46e4d628d
📒 Files selected for processing (3)
queue/src/boot.rsqueue/src/functions.rsqueue/src/main.rs
| if let Some(seed) = seed.as_ref() { | ||
| if seed.adapter != config.adapter { | ||
| tracing::warn!( | ||
| seed_adapter = ?seed.adapter, | ||
| stored_adapter = ?config.adapter, | ||
| "--config seed adapter ignored; the stored configuration is authoritative" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Only compare adapters when the seed specifies one.
seed can be present while seed.adapter is None. In that case, this emits an “adapter ignored” warning when the stored configuration has an adapter, even though --config did not provide one. Gate the comparison on seed.adapter.is_some().
Proposed fix
if let Some(seed) = seed.as_ref() {
- if seed.adapter != config.adapter {
+ if let Some(seed_adapter) = seed.adapter.as_ref() {
+ if Some(seed_adapter) != config.adapter.as_ref() {
tracing::warn!(
- seed_adapter = ?seed.adapter,
+ seed_adapter = ?seed_adapter,
stored_adapter = ?config.adapter,
"--config seed adapter ignored; the stored configuration is authoritative"
);
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(seed) = seed.as_ref() { | |
| if seed.adapter != config.adapter { | |
| tracing::warn!( | |
| seed_adapter = ?seed.adapter, | |
| stored_adapter = ?config.adapter, | |
| "--config seed adapter ignored; the stored configuration is authoritative" | |
| ); | |
| } | |
| } | |
| if let Some(seed) = seed.as_ref() { | |
| if let Some(seed_adapter) = seed.adapter.as_ref() { | |
| if Some(seed_adapter) != config.adapter.as_ref() { | |
| tracing::warn!( | |
| seed_adapter = ?seed_adapter, | |
| stored_adapter = ?config.adapter, | |
| "--config seed adapter ignored; the stored configuration is authoritative" | |
| ); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@queue/src/main.rs` around lines 92 - 100, Update the adapter comparison in
the seed validation block to run only when seed.adapter is Some, while
preserving the existing mismatch warning and stored-configuration authority
behavior for explicitly provided adapters.
1 participant
Summary
The builtin queue adapter silently falls back to the in-memory store when the stored configuration has no
adapterentry — even when the--configseed explicitly asks forfile_basedpersistence. The only boot log line (loaded seed config path=…) reads as if the seed took effect, so a stack can run without durability and nothing says so. Hit this on a live dev stack: the store file had been frozen for days while turns ran through an in-memory store.store="file_based" path=…orstore="in_memory"(with a note that jobs don't survive restarts).--configseed specifies an adapter that differs from the authoritative stored value, boot logs a WARN showing both, instead of discarding the seed silently. The stored value still wins — configuration-worker authority is unchanged.engine::queue::topic_statsnow returns thedelivered/failedcounters the store already tracks (additive fields; single construction site, no existing consumers read them).Test plan
cargo testinqueue/— 130 passed, including the extendedlist_and_stats_return_adapter_stateassertion ondelivered/failedadapter: WARN +store="in_memory"lines appearconfiguration::setadding the adapter: hot-swap logsstore="file_based" path=./data/harness-queue, store file resumes updating (revision advances, delivered increments)engine::queue::topic_statsreturnsdelivered/failedSummary by CodeRabbit