fix: add serverless backfill #3588

MasterPtato · 2025-12-04T20:53:34Z

No description provided.

vercel · 2025-12-04T20:53:48Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Dec 4, 2025 8:55pm

3 Skipped Deployments

Project	Deployment	Preview	Updated (UTC)
rivet-cloud	Ignored		Dec 4, 2025 8:55pm
rivet-inspector	Ignored	Preview	Dec 4, 2025 8:55pm
rivet-site	Ignored		Dec 4, 2025 8:55pm

MasterPtato · 2025-12-04T20:53:49Z

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: add serverless backfill #3588 👈 (View in Graphite)
chore: fix runner config creation #3580
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.

pkg-pr-new · 2025-12-04T20:55:56Z

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 9284eb3

claude · 2025-12-04T20:56:14Z

Pull Request Review: Add Serverless Backfill

Overview

This PR adds a serverless backfill service to ensure runner pool workflows are properly initialized for all existing serverless configurations on engine startup. It also removes the deprecated singleton deployment configuration.

Code Quality & Best Practices

✅ Strengths

Proper service type: Using ServiceKind::Oneshot is appropriate for a backfill operation that should run once at startup
Transaction isolation: Correctly uses Snapshot isolation level to prevent conflicts with concurrent updates
Structured logging: Follows the repository's structured logging pattern with tracing::debug\! and field formatting
Workflow deduplication: Uses .unique() to prevent duplicate workflows from being created
Dependency management: Correctly adds the workspace dependency

⚠️ Areas for Improvement

1. Missing early exit check (engine/packages/serverless-backfill/src/lib.rs:45-46)

The code checks if serverless_data.is_empty() and returns early, but this check happens after fetching runner configs. The early exit should occur before the database operation:

// Current (lines 20-54):
let serverless_data = ctx.udb()?.run(|tx| { /* ... */ }).await?;

let runner_configs = ctx.op(pegboard::ops::runner_config::get::Input {
    runners: serverless_data.iter()...
}).await?;

if serverless_data.is_empty() {
    return Ok(());
}

Should be:

let serverless_data = ctx.udb()?.run(|tx| { /* ... */ }).await?;

if serverless_data.is_empty() {
    return Ok(());
}

let runner_configs = ctx.op(pegboard::ops::runner_config::get::Input {
    runners: serverless_data.iter()...
}).await?;

2. Incomplete validation logic (engine/packages/serverless-backfill/src/lib.rs:56-67)

The validation at lines 56-67 only checks if any runner config matches the namespace_id, but doesn't verify the specific runner_name matches. This could cause workflows to be dispatched for deleted runners if another runner exists in the same namespace:

// Current:
if \!runner_configs.iter().any(|rc| rc.namespace_id == key.namespace_id) {
    // skip
}

Should be:

if \!runner_configs.iter().any(|rc|
    rc.namespace_id == key.namespace_id &&
    rc.runner_name == key.runner_name
) {
    tracing::debug\!(
        namespace_id=?key.namespace_id,
        runner_name=?key.runner_name,
        "runner config not found, likely deleted"
    );
    continue;
}

3. Import organization (engine/packages/serverless-backfill/src/lib.rs:5)

Using wildcard import for IsolationLevel is less clear than explicit imports:

// Current:
use universaldb::utils::IsolationLevel::*;

// Better:
use universaldb::utils::IsolationLevel;
// Then use: IsolationLevel::Snapshot

4. Cargo.toml formatting inconsistency (Cargo.toml)

The PR changes formatting in unrelated workspace dependencies (removing spaces in feature arrays). While not functionally harmful, these should be separate commits:

Line 154: features = ["derive","cargo"] (removed spaces)
Line 90: features = ["build","cargo","rustc"] (removed spaces)

Potential Bugs

🐛 Critical: The validation logic bug (#2 above) could cause workflows to be dispatched for runners that have been deleted, as long as any other runner exists in the same namespace.

Performance Considerations

✅ Good:

Uses StreamingMode::WantAll appropriately for bulk operations
Snapshot isolation prevents blocking concurrent operations
Oneshot service runs once and exits

⚠️ Minor optimization: The early exit check should happen before the runner_configs fetch to avoid unnecessary database operations when there's no data to backfill.

Security Concerns

✅ No security issues identified. The code:

Uses proper transaction isolation
Doesn't expose sensitive data in logs
Follows the repository's error handling patterns

Test Coverage

⚠️ Missing: No tests are included for the new serverless-backfill package. Consider adding:

Unit test for empty serverless_data case
Unit test for runner config validation logic
Integration test for workflow dispatch behavior

Infrastructure Changes

✅ Good: Removing the singleton deployment (k8s/engine/06-rivet-engine-singleton-deployment.yaml) is appropriate since this functionality is now handled by the Oneshot service.

Summary

Recommendation: Request changes for the validation logic bug (#2) before merging. The other issues are minor improvements.

Priority fixes:

🔴 Fix runner name validation (critical bug)
🟡 Move early exit check before runner_configs fetch
🟢 Consider adding tests

The overall approach is sound and follows repository conventions well.

claude · 2025-12-04T20:56:54Z

PR Review: Add Serverless Backfill

Overview

This PR adds a new oneshot service that backfills serverless runner pool workflows on engine startup. It also removes the now-unnecessary singleton deployment infrastructure from Kubernetes.

Positive Aspects

Clean Architecture: The new serverless-backfill package follows existing patterns well
Proper Oneshot Service: Using ServiceKind::Oneshot is appropriate - it runs once at startup and exits
Efficient Database Query: Uses snapshot isolation to prevent conflicts while reading desired slots
Good Error Handling: Gracefully handles missing runner configs (likely deleted)
Infrastructure Simplification: Removing the singleton deployment reduces operational complexity

Issues and Concerns

1. Critical: Runner Config Matching Bug (engine/packages/serverless-backfill/src/lib.rs:63-73)

The logic for checking if a runner config exists has a bug. It only checks if ANY runner config exists for the namespace, not if the SPECIFIC runner config exists. This will skip dispatching workflows even when the correct runner config exists, as long as ANY other runner config for that namespace is present.

Fix: Check both namespace_id AND runner_name in the any() predicate at line 65.

2. Missing Dependency Alias (engine/packages/serverless-backfill/Cargo.toml:8)

Using gas.workspace = true but the workspace dependency is called gasoline, not gas. This will fail to compile.

3. Missing Error Context (engine/packages/serverless-backfill/src/lib.rs:9-18)

Multiple fallible operations lack error context for better debugging.

Performance Considerations

Batch Workflow Dispatch: Currently dispatches workflows sequentially in a loop. For large numbers of runner configs, this could be slow. Consider using futures::stream to dispatch multiple workflows concurrently.
Large Result Set Handling: Using StreamingMode::WantAll could cause memory issues with many serverless configurations.

Security Considerations

No security concerns identified. The service uses snapshot isolation for safe reads, properly validates runner configs before dispatching workflows, and uses workflow deduplication.

Testing

Missing: No tests for the new package. Add unit tests for the runner config matching logic (especially given the bug found) and integration tests to verify workflows are dispatched correctly.

Style and Convention Compliance

Follows CLAUDE.md conventions: uses structured logging with tracing, lowercase log messages, uses workspace dependencies appropriately, follows existing error patterns.

Summary

Severity of Issues:

Blocker: Runner config matching bug will cause incorrect behavior
High: Missing dependency alias will fail to compile
Low: Missing error context reduces debuggability
Low: Missing tests reduces confidence

Recommendation: Fix issues 1 and 2 before merging. Consider adding tests to prevent regressions.

Generated with Claude Code

MasterPtato requested a review from NathanFlurry December 4, 2025 20:53

fix: add serverless backfill

9284eb3

MasterPtato force-pushed the 12-04-fix_add_serverless_backfill branch from b9c23a8 to 9284eb3 Compare December 4, 2025 20:53

MasterPtato mentioned this pull request Dec 4, 2025

chore: fix runner config creation #3580

Open

vercel bot deployed to Preview – rivetkit-serverless December 4, 2025 20:55 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: add serverless backfill #3588

fix: add serverless backfill #3588

Uh oh!

MasterPtato commented Dec 4, 2025

Uh oh!

vercel bot commented Dec 4, 2025 •

edited

Loading

Uh oh!

MasterPtato commented Dec 4, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Dec 4, 2025 •

edited

Loading

Uh oh!

claude bot commented Dec 4, 2025

Uh oh!

claude bot commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: add serverless backfill #3588

Are you sure you want to change the base?

fix: add serverless backfill #3588

Uh oh!

Conversation

MasterPtato commented Dec 4, 2025

Uh oh!

vercel bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MasterPtato commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

pkg-pr-new bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Dec 4, 2025

Pull Request Review: Add Serverless Backfill

Overview

Code Quality & Best Practices

✅ Strengths

⚠️ Areas for Improvement

Potential Bugs

Performance Considerations

Security Concerns

Test Coverage

Infrastructure Changes

Summary

Uh oh!

claude bot commented Dec 4, 2025

PR Review: Add Serverless Backfill

Overview

Positive Aspects

Issues and Concerns

1. Critical: Runner Config Matching Bug (engine/packages/serverless-backfill/src/lib.rs:63-73)

2. Missing Dependency Alias (engine/packages/serverless-backfill/Cargo.toml:8)

3. Missing Error Context (engine/packages/serverless-backfill/src/lib.rs:9-18)

Performance Considerations

Security Considerations

Testing

Style and Convention Compliance

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Dec 4, 2025 •

edited

Loading

MasterPtato commented Dec 4, 2025 •

edited

Loading

pkg-pr-new bot commented Dec 4, 2025 •

edited

Loading