Skip to content

fix(sqlite): quiet startup database miss#5005

Merged
NathanFlurry merged 1 commit into
mainfrom
sqlite/quiet-startup-db-miss
May 9, 2026
Merged

fix(sqlite): quiet startup database miss#5005
NathanFlurry merged 1 commit into
mainfrom
sqlite/quiet-startup-db-miss

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

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 9, 2026

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

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web May 9, 2026 at 7:33 am
website 😴 Sleeping (View Logs) Web May 9, 2026 at 3:27 am
frontend-inspector ❌ Build Failed (View Logs) Web May 9, 2026 at 3:18 am
mcp-hub ✅ Success (View Logs) Web May 9, 2026 at 3:18 am
frontend-cloud ❌ Build Failed (View Logs) Web May 9, 2026 at 3:18 am
ladle ❌ Build Failed (View Logs) Web May 9, 2026 at 3:18 am

@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5005 May 9, 2026 03:17 Destroyed
Copy link
Copy Markdown
Member Author

NathanFlurry commented May 9, 2026

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 9, 2026

PR #5005 Review: fix(sqlite): quiet startup database miss

Overview

This PR downgrades a noisy tracing::error! to tracing::debug! for the case where a SqliteGetPagesRequest returns DatabaseNotFound during actor startup (when both expected_generation and expected_head_txid are None). This is an expected condition - a brand-new actor querying pages before it has ever committed state - not a real error.

The response to the client is unchanged (SqliteErrorResponse is still returned), so VFS behavior is unaffected. This is a pure observability fix.


Code Quality

The implementation is clean and follows existing conventions:

  • is_startup_database_miss is well-named and appropriately extracted as a private helper.

  • Structured tracing fields are consistent with surrounding code.

  • The matches! macro usage is idiomatic.

One minor observation: expected_generation is currently always None at all three call sites in depot-client/src/vfs.rs (hardcoded). So expected_generation.is_none() is always true in practice and the condition is effectively gated only on expected_head_txid.is_none() && DatabaseNotFound. Keeping the generation check in is good defensive forward-compatibility - if generation tracking is added to the VFS later, the predicate will naturally tighten without needing a revisit.


Potential Issues

  • Debug log omits ?err: The debug branch drops the error from the log, which is intentional and fine since we know it is DatabaseNotFound. No issue here.

  • Error response is still sent: The function still returns SqliteErrorResponse even for the startup-miss case. This is correct - the VFS client needs the DatabaseNotFound signal to initialize a fresh database. No behavior change is introduced.

  • No new enum arm exhaustion risk: The predicate uses matches! against a specific variant, not a catch-all, so new SqliteStorageError variants will not silently change this behavior.


Suggestions

  1. Consider tracing::trace! instead of debug!: This event fires on every page read for a fresh actor. At scale, debug! may still generate significant output if debug logging is enabled broadly. trace! would be more appropriate for a signal that is completely expected on every cold start.

  2. No test added: Not strictly required for a log-level change, but a small unit test for is_startup_database_miss - covering the four (expected_generation, expected_head_txid) combinations with DatabaseNotFound vs. another error - would lock in the predicate semantics cheaply. Optional given the low risk.


Summary

Correct, well-scoped fix. The logic is sound, the helper is clearly named, and the response behavior is unchanged. The only real discussion point is whether debug! should be trace! - otherwise this is good to go.

@NathanFlurry NathanFlurry changed the base branch from rivetkit-channels/use-unbounded-channels to graphite-base/5005 May 9, 2026 03:29
@NathanFlurry NathanFlurry force-pushed the sqlite/quiet-startup-db-miss branch from d764db4 to 49f9288 Compare May 9, 2026 03:29
@NathanFlurry NathanFlurry changed the base branch from graphite-base/5005 to metrics/split-server-crate May 9, 2026 03:29
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5005 May 9, 2026 03:29 Destroyed
@NathanFlurry NathanFlurry changed the base branch from metrics/split-server-crate to graphite-base/5005 May 9, 2026 03:38
@NathanFlurry NathanFlurry force-pushed the graphite-base/5005 branch from 30f3c06 to f63add8 Compare May 9, 2026 05:57
@NathanFlurry NathanFlurry force-pushed the sqlite/quiet-startup-db-miss branch from 49f9288 to e8f7214 Compare May 9, 2026 05:57
@NathanFlurry NathanFlurry changed the base branch from graphite-base/5005 to metrics/split-server-crate May 9, 2026 05:57
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5005 May 9, 2026 05:57 Destroyed
@NathanFlurry NathanFlurry changed the base branch from metrics/split-server-crate to graphite-base/5005 May 9, 2026 05:57
@NathanFlurry NathanFlurry force-pushed the sqlite/quiet-startup-db-miss branch from e8f7214 to 4e070ea Compare May 9, 2026 06:24
@NathanFlurry NathanFlurry force-pushed the graphite-base/5005 branch from f63add8 to 71e98d4 Compare May 9, 2026 06:24
@NathanFlurry NathanFlurry changed the base branch from graphite-base/5005 to metrics/split-server-crate May 9, 2026 06:24
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5005 May 9, 2026 06:24 Destroyed
@NathanFlurry NathanFlurry marked this pull request as ready for review May 9, 2026 06:51
@NathanFlurry NathanFlurry force-pushed the metrics/split-server-crate branch from 71e98d4 to fd0ce61 Compare May 9, 2026 07:33
@NathanFlurry NathanFlurry force-pushed the sqlite/quiet-startup-db-miss branch from 4e070ea to 16c6a31 Compare May 9, 2026 07:33
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5005 May 9, 2026 07:33 Destroyed
Base automatically changed from metrics/split-server-crate to main May 9, 2026 07:50
@NathanFlurry NathanFlurry merged commit 16c6a31 into main May 9, 2026
9 of 13 checks passed
@NathanFlurry NathanFlurry deleted the sqlite/quiet-startup-db-miss branch May 9, 2026 07:50
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