Skip to content

DO NOT MERGE: queue diagnose#5050

Draft
abcxff wants to merge 1 commit into
05-13-fix_rivetkit_keep_actor_alive_while_workflow_waits_for_queue_messagesfrom
05-12-do_not_merge_queue_diagnose
Draft

DO NOT MERGE: queue diagnose#5050
abcxff wants to merge 1 commit into
05-13-fix_rivetkit_keep_actor_alive_while_workflow_waits_for_queue_messagesfrom
05-12-do_not_merge_queue_diagnose

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented May 12, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 12, 2026

🚅 Deployed to the rivet-pr-5050 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-inspector ❌ Build Failed (View Logs) Web May 13, 2026 at 2:59 am
frontend-cloud ❌ Build Failed (View Logs) Web May 13, 2026 at 2:57 am
kitchen-sink ❌ Build Failed (View Logs) Web May 13, 2026 at 2:57 am
ladle ❌ Build Failed (View Logs) Web May 12, 2026 at 5:53 pm
mcp-hub ✅ Success (View Logs) Web May 12, 2026 at 5:48 pm
website ❌ Build Failed (View Logs) Web May 12, 2026 at 5:47 pm

@abcxff abcxff mentioned this pull request May 12, 2026
11 tasks
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 12, 2026

PR #5050 Review: DO NOT MERGE — Queue Diagnostics

Note: This is a DRAFT PR explicitly marked "DO NOT MERGE". This review treats it as a diagnostic/investigation branch.


Overview

This PR adds temporary DEBUG_QUEUE-prefixed instrumentation across the actor queue implementation to diagnose queue behavior. Changes span:

  • rivetkit-rust/packages/rivetkit-core/src/actor/queue.rs — Rust-side warn-level trace points
  • rivetkit-typescript/packages/rivetkit/src/registry/native.ts — TS-side debug logs around nextBatch
  • rivetkit-typescript/packages/workflow-engine/src/context.ts — debug logs in executeQueueNextBatch

There is one incidental correctness fix: list_message_entries was refactored from a chained .await.context() return into a bound variable + explicit Ok(entries), which also makes the new log line possible.


Issues That Must Be Fixed Before Any Merge

1. Wrong log level in Rust (all tracing::warn! should be tracing::debug! or tracing::trace!)

All 7 Rust debug statements use tracing::warn!. This is the wrong level for diagnostic traces:

  • warn is visible in all production deployments by default.
  • It will fire on every enqueue_message, next_batch, try_receive_batch, and list_message_entries call, flooding operator logs.
  • Per CLAUDE.md and project conventions, use tracing::debug! or tracing::trace! for diagnostic instrumentation.

The TypeScript side correctly uses .debug() and "debug" level — the Rust side should match.

2. DEBUG_QUEUE prefix in log messages

The DEBUG_QUEUE prefix signals temporary instrumentation. Remove before any permanent merge, or convert to proper structured fields (e.g., component = "queue").


Code Quality Notes

  • has_preload variable (queue.rs:588): Clean addition. Clearer than re-calling .is_some() in the log.
  • list_message_entries refactor (queue.rs:767-787): The extraction into a named variable is a valid improvement regardless of the debug logging. The original chained .await.context() was correct but opaque; the new form is easier to read and lets the compiler surface the type.
  • TypeScript log style: Consistent with the existing logger().debug() and this.log("debug", ...) patterns. No issues.
  • Structured fields: The Rust logs follow CLAUDE.md conventions (structured fields, lowercase messages, no string interpolation). Good.

Summary

Area Status
Draft / DO NOT MERGE marker Expected for diagnostic branch
Rust log level (warn instead of debug) Must fix before merge
DEBUG_QUEUE prefixes in messages Must remove before merge
TS log levels Correct
Incidental list_message_entries refactor Clean, keep it
Test coverage N/A (diagnostic-only change)
Security / performance No concerns beyond log spam from warn level

Bottom line: The diagnostic intent is clear and the instrumentation covers the right call sites. Before this could ever land (if the investigation yields a follow-up fix PR), downgrade all Rust tracing::warn! calls to tracing::debug! and strip the DEBUG_QUEUE prefixes.

@abcxff abcxff force-pushed the 05-09-fix_workflow-engine_only_commit_step_state_after_success branch from 2c029c2 to 9b64fa5 Compare May 12, 2026 18:03
@abcxff abcxff force-pushed the 05-12-do_not_merge_queue_diagnose branch from 90567e2 to 8545743 Compare May 12, 2026 18:03
@abcxff abcxff force-pushed the 05-12-do_not_merge_queue_diagnose branch from 8545743 to f7733df Compare May 12, 2026 18:44
@abcxff abcxff force-pushed the 05-09-fix_workflow-engine_only_commit_step_state_after_success branch from 9b64fa5 to 0eb8f51 Compare May 12, 2026 18:44
@abcxff abcxff force-pushed the 05-12-do_not_merge_queue_diagnose branch from f7733df to 1474a34 Compare May 12, 2026 20:01
@abcxff abcxff changed the base branch from 05-09-fix_workflow-engine_only_commit_step_state_after_success to graphite-base/5050 May 13, 2026 02:56
@abcxff abcxff force-pushed the 05-12-do_not_merge_queue_diagnose branch from 1474a34 to 0a70a7c Compare May 13, 2026 02:56
@abcxff abcxff changed the base branch from graphite-base/5050 to 05-13-fix_rivetkit_keep_actor_alive_while_workflow_waits_for_queue_messages May 13, 2026 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant