Skip to content

chore: add log for runners with no alloc idx#4316

Merged
NathanFlurry merged 9 commits intomainfrom
02-25-chore_add_log_for_runners_with_no_alloc_idx
Feb 27, 2026
Merged

chore: add log for runners with no alloc idx#4316
NathanFlurry merged 9 commits intomainfrom
02-25-chore_add_log_for_runners_with_no_alloc_idx

Conversation

@MasterPtato
Copy link
Contributor

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

railway-app bot commented Feb 26, 2026

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

Service Status Web Updated (UTC)
website 😴 Sleeping (View Logs) Web Feb 26, 2026 at 1:49 am
frontend-inspector ❌ Build Failed (View Logs) Web Feb 26, 2026 at 1:42 am
mcp-hub ✅ Success (View Logs) Web Feb 26, 2026 at 1:42 am
frontend-cloud ❌ Build Failed (View Logs) Web Feb 26, 2026 at 1:42 am
ladle ❌ Build Failed (View Logs) Web Feb 26, 2026 at 1:41 am

Copy link
Contributor Author

MasterPtato commented Feb 26, 2026

@MasterPtato MasterPtato mentioned this pull request Feb 26, 2026
11 tasks
@claude
Copy link

claude bot commented Feb 26, 2026

PR Review: chore: add log for runners with no alloc idx

Summary: This PR adds a temporary diagnostic branch in check_expired to detect runners that are missing their RunnerAllocIdxKey entry when they should have one (not expired, not draining). It also threads draining state into the CheckExpiredInput struct with #[serde(default)] for backwards-compatibility.


Bug: Tuple Mismatch in tokio::try_join! (Compile Error)

The most critical issue is in the new else if !input.draining branch. The tokio::try_join! call produces a 6-tuple, but the binding destructure lists 7 names:

let (
    namespace_id_entry,
    name_entry,
    version_entry,
    remaining_slots_entry,
    total_slots_entry,
    protocol_version_entry,  // ← 7th name, no corresponding entry in try_join!
    last_ping_ts_entry,
) = tokio::try_join!(
    tx.read_opt(&namespace_id_key, Snapshot),    // 1
    tx.read_opt(&name_key, Snapshot),            // 2
    tx.read_opt(&version_key, Snapshot),         // 3
    tx.read_opt(&remaining_slots_key, Snapshot), // 4
    tx.read_opt(&total_slots_key, Snapshot),     // 5
    tx.read_opt(&last_ping_ts_key, Snapshot),    // 6
)?;

This is a compile error — Rust cannot destructure a 6-tuple into 7 binding names. protocol_version_entry also never appears in the subsequent let ... else match, confirming it's a leftover from a copy-paste (protocol version is not a parameter of RunnerAllocIdxKey::new and isn't needed here).

Fix: Remove protocol_version_entry from the destructure:

let (
    namespace_id_entry,
    name_entry,
    version_entry,
    remaining_slots_entry,
    total_slots_entry,
    last_ping_ts_entry,
) = tokio::try_join!(...)?;

Minor: Potential Division by Zero

let remaining_millislots = (remaining_slots * 1000) / total_slots;

If total_slots is ever 0 this panics. If it's guaranteed to be non-zero by the time a runner has initiated (the Some(total_slots) guard only ensures it was stored, not that it's non-zero), consider a guard or a saturating division. This mirrors the same expression at line 414 in the existing connect_runner2 path, so it's consistent, but worth noting.


Positives

  • #[serde(default)] on draining is the right approach for activity input backwards-compatibility.
  • The TODO comment clearly marks this as temporary scaffolding.
  • The tracing::debug! early-return log is helpful for the "not initiated yet" case.
  • The warning log message is informative and well-structured.
  • Gating the check on !input.draining is correct — draining runners legitimately may not have an alloc index entry.

@MasterPtato MasterPtato force-pushed the 02-25-chore_add_log_for_runners_with_no_alloc_idx branch from acdd4c7 to fb17f0a Compare February 26, 2026 02:14
@MasterPtato MasterPtato force-pushed the 02-25-fix_ups_upgrade_nats_fix_chunking_add_chunking_tests branch 2 times, most recently from e0b072a to e39972b Compare February 26, 2026 19:42
@MasterPtato MasterPtato force-pushed the 02-25-chore_add_log_for_runners_with_no_alloc_idx branch from fb17f0a to 5e58e8c Compare February 26, 2026 19:42
@MasterPtato MasterPtato force-pushed the 02-25-fix_ups_upgrade_nats_fix_chunking_add_chunking_tests branch from e39972b to 409fdf2 Compare February 27, 2026 00:12
@MasterPtato MasterPtato force-pushed the 02-25-chore_add_log_for_runners_with_no_alloc_idx branch from 5e58e8c to a9c0244 Compare February 27, 2026 00:12
@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 27, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4316

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4316

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4316

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4316

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4316

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4316

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4316

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4316

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4316

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4316

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4316

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4316

commit: a9c0244

Base automatically changed from 02-25-fix_ups_upgrade_nats_fix_chunking_add_chunking_tests to main February 27, 2026 00:30
@NathanFlurry NathanFlurry merged commit d4001d0 into main Feb 27, 2026
16 of 25 checks passed
@NathanFlurry NathanFlurry deleted the 02-25-chore_add_log_for_runners_with_no_alloc_idx branch February 27, 2026 00:31
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.

2 participants