fix(serverless): drain runners after url/headers change

[MasterPtato]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Dec 2, 2025 7:34pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Dec 2, 2025 7:34pm
rivet-inspector	Ignored	Preview		Dec 2, 2025 7:34pm
rivet-site	Ignored	Preview		Dec 2, 2025 7:34pm

[MasterPtato]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(serverless): misc pr fixes #3576
• fix(serverless): fix connection wf lifecycle #3573
• fix(serverless): drain runners after url/headers change #3571 👈 (View in Graphite)
• feat(pegboard): move pegboard serverless to wf #3416 : 1 other dependent PR (#3450 )
• fix(runtime): evict runners on SIGTERM, add periodic shutdown progress messages #3550
• fix(pb): implement events and commands for actor wfs #3543
• fix: make all uses of protocol handle v4 <-> v3 #3528
• fix(pb): simplify runner wf #3483
• fix(gas): fix loop forgotten bug due to concurrency #3482
• fix(pb): rewrite runner wf to handle batch signals #3480
• fix(gas): fix batch listen, fix history for graceful signal send in workflows #3479
• feat(gas): add batch listen #3478
• chore(gas): add test for history with holes #3477
• chore: change streamingmode to exact for ranges with limits #3476
• fix(gas): optimize loops #3475
• fix: parallelize publish signal awaits #3523
• chore: fix diff between ee, fix plural api query params #3524
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: fix(serverless): drain runners after url/headers change

Summary

This PR implements automatic draining of serverless runners when their configuration (URL or headers) changes. The implementation tracks a hash of these details and triggers drain for runners with outdated hashes.

---

Code Quality & Best Practices

Strengths:

• Good separation of concerns with the new RunnerState struct tracking both workflow ID and config hash
• Proper use of structured logging with #[tracing::instrument(skip_all)] attributes
• Follows the codebase error handling patterns using anyhow
• Import ordering follows CLAUDE.md guidelines (std imports first)

Issues:

1. Hash collision risk (pool.rs:217-223)

   • DefaultHasher is explicitly not guaranteed to be stable across Rust versions or platforms
   • For configuration change detection, you should use a stable hash implementation
   • This could cause false-positive drains after Rust upgrades

2. Missing error context (pool.rs:63-68) - If signaling fails, the runner will be removed from state but may not actually drain. Consider logging which runners are being drained for observability.

---

Potential Bugs

1. Race condition during hash change (pool.rs:56-68)

   • When configuration changes, all runners with old hash are drained immediately
   • If many runners exist and all drain simultaneously, there could be a service disruption
   • Consider: Implement gradual draining to maintain availability

2. Signal delivery (runner.rs:43-51) - Both signal and message are sent for Drain. The connection workflow handles both independently which is good.

---

Performance Considerations

1. Hashing on every iteration (pool.rs:217-223) - Hash is computed on every pool loop iteration. Since URL and headers rarely change, consider caching.

2. Partition creates two vectors (pool.rs:58-60) - For large runner counts, this could be expensive. Likely acceptable given typical counts.

---

Test Coverage

Missing:

• No tests found for the new hash-based draining logic
• No tests verifying behavior when configuration changes

Recommendations:

1. Add integration test simulating URL change and verifying all old runners drain
2. Add test for header change detection
3. Test gradual migration scenario

---

Summary Recommendations

Must Fix:

1. ⚠️ Consider using a stable hash implementation or document the DefaultHasher limitations
2. ⚠️ Add logging when draining runners due to config changes for observability

Should Fix:

1. Add test coverage for configuration change scenarios
2. Consider gradual draining to prevent service disruption

Nice to Have:

1. Emit metrics for configuration-driven drains
2. Optimize hash computation

---

Verdict

The implementation is solid and follows the codebase conventions well. Main concerns are around hash stability and lack of test coverage. The logic correctly handles configuration changes but could benefit from better observability.

Recommended action: Approve with minor fixes suggested (add logging and document hash limitations).

[pkg-pr-new]

More templates

• example-ai-agent
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-cloudflare-workers-inline-client
• example-counter
• example-counter-next-js
• example-counter-serverless
• example-cursors
• example-cursors-raw-websocket
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• quickstart-actions
• quickstart-cross-actor-actions
• quickstart-multi-region
• quickstart-raw-websockets
• quickstart-realtime
• quickstart-scheduling
• quickstart-state
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• starter
• example-stream
• example-trpc

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 72f5f46

[graphite-app]

Merge activity

• Dec 3, 12:33 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Dec 3, 12:34 AM UTC: CI is running for this pull request on a draft pull request (#3577) due to your merge queue CI optimization settings.
• Dec 3, 12:34 AM UTC: Merged by the Graphite merge queue via draft PR: #3577.