Skip to content

docs(rivetkit): raise serverless start payload limit#4920

Closed
NathanFlurry wants to merge 1 commit intodriver-test-complaints/sqlite-memory-soak-harnessfrom
driver-test/complaints/raise-serverless-start-payload-limit
Closed

docs(rivetkit): raise serverless start payload limit#4920
NathanFlurry wants to merge 1 commit intodriver-test-complaints/sqlite-memory-soak-harnessfrom
driver-test/complaints/raise-serverless-start-payload-limit

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member Author

NathanFlurry commented May 4, 2026

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

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

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

Code Review

PR: docs(rivetkit): raise serverless start payload limit
Status: Closed

Overview

This PR raises the default maxStartPayloadBytes for serverless mode from 1 MiB to 16 MiB to accommodate KV preloading and SQLite startup page preloads serialized into the /api/rivet/start request body. It also adds glibc memory allocator tuning defaults for the spawned engine subprocess, and updates documentation.


Code Quality

Constant extraction (serverless.ts)

  • Extracting DEFAULT_SERVERLESS_MAX_START_PAYLOAD_BYTES as a named export and using it in the schema description via template literal is the right call. The description now stays in sync with the actual default automatically.

Memory allocator tuning (engine_process.rs)

  • The set_default_allocator_env helper is clean and correctly guards with is_none() so operators can override.
  • MALLOC_ARENA_MAX=2 and MALLOC_TRIM_THRESHOLD_=131072 (128 KiB) are standard glibc defaults for memory-constrained or memory-fragmentation-sensitive workloads.

Potential Issues

Platform portability of glibc env vars

MALLOC_ARENA_MAX and MALLOC_TRIM_THRESHOLD_ are glibc-specific and only have an effect on Linux with glibc. They are silently ignored on macOS and Windows. This isn't a bug since they're just ignored, but a comment explaining this would prevent future confusion:

// glibc-specific: reduces arena fragmentation on Linux; ignored on other platforms
fn set_default_allocator_env(command: &mut Command) {

dev:serverless script and env var interaction

"dev:serverless": "MALLOC_ARENA_MAX=2 MALLOC_TRIM_THRESHOLD_=131072 RIVET_RUN_ENGINE=1 ..."

Setting these vars in the shell environment means the node server process inherits them. When the engine subprocess is spawned, set_default_allocator_env will find them already set via is_none() and correctly skip re-setting them. This is fine, but the intent (applying them to the node process too) could be worth a comment since it looks redundant next to the Rust-side defaults.

No schema validation bounds on maxStartPayloadBytes

The field has no .min() or .max() constraint. With a 16 MiB default and the operational significance of this value, a minimum floor (e.g., .min(1_048_576)) would prevent silent misconfiguration like maxStartPayloadBytes: 0. This was pre-existing, not introduced here, but the raised default makes it more salient.


Documentation

  • The limits.mdx addition is accurate and gives the right context (why 16 MiB vs the 1 MiB preload budget).
  • The sentence is long — consider splitting after "...alongside the start command." for readability.
  • The production checklist addition is practical and actionable. Good inclusion.

Summary

The change is straightforward and well-motivated. The constant extraction and the set_default_allocator_env guard are both done correctly. The main gaps are:

  1. Missing a comment on glibc-specific env vars (minor).
  2. No min/max validation on maxStartPayloadBytes (pre-existing, worth a follow-up).
  3. The dev:serverless script applying env vars to the parent node process vs. the subprocess is slightly surprising without a comment.

Overall a clean, low-risk change.

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