Skip to content

Bump redb to 4.4: fix integrity failure after compaction (main CI red since #136) - #140

Merged
sujitn merged 1 commit into
mainfrom
fix-redb-compaction
Oct 22, 2026
Merged

sujitn merged 1 commit into
mainfrom
fix-redb-compaction

Conversation

@sujitn

@sujitn sujitn commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Main's Test job has failed since #136 merged: convex-ext-redb/tests/recovery.rs panics with "reconciled fixture must pass integrity without another repair". Every PR inherits the failure, including #139.

Cause

The cause is redb 4.1.0, not our code:

  • After convex-redb compact and repair the database checks clean.
  • The first write transaction after that makes check_integrity() report a repair. Even an empty begin_write().commit() does it.
  • The commit after that is clean again.

What triggered it: today the test passes at db1d144 and fails at 8803b4c, with the same toolchain. #136 changed the recovery fixture data in between, and I take that to have shifted the file layout; I haven't traced the exact mechanism. The test asserts integrity after reopening the storage, and RedbRuntimeStore::new commits on every open, so it fails.

Operators would hit the same thing: compact, start the server once, and backup_database refuses with "database required repair".

redb 4.4.0 fixes it ("Fix compact() leaving free regions at the end of the file, when the compaction emptied more of them than it made commits"). On 4.4.0 the empty-commit probe stays clean and all recovery tests pass.

The upgrade also brings two fixes on the same backup and repair path:

  • 4.3.0: a crash shortly after a commit could silently roll that commit back during recovery, if check_integrity() had previously repaired the database.
  • 4.4.0: check_integrity() could report a false repair after an aborted transaction had grown the file.

Change

  • Cargo.lock: redb 4.1.0 → 4.4.0. Its MSRV is 1.90; CI uses 1.97.1.
  • redb = "4.4" in the workspace and crate manifests, so the fix is the minimum version.
  • curve_store.rs: two string literals held raw NUL bytes, so git and GitHub showed the file as binary and its diffs were hidden in review. They are now \0 escapes, with the same value.

No .redb files are checked in. A scratch program checked files written by 4.1.0:

  • redb 4.1.0 wrote string-keyed and integer-keyed tables (10,000 rows each).
  • redb 4.4.0 then opened the file, checked it clean, read the rows back and ran an insert and a remove.
  • It checked clean again, compacted, committed once, and checked clean again.

Verification (WSL)

  • cargo test -p convex-ext-redb -p convex-server --no-fail-fast: 34 test binaries, 0 failed. Before the bump, recovery failed locally exactly as in CI.
  • cargo clippy -p convex-ext-redb --all-targets -D warnings: clean.
  • cargo fmt --all --check: clean.
  • MSRV: workspace rust-version is 1.95 and redb 4.4 needs 1.90.

#139 goes green once this merges and main is merged into it.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the supported version range for the storage component. No user-facing changes are included.

redb 4.1.0 left free regions after compact(), so the first commit after
compact and repair made check_integrity() report a repair. That broke the
recovery tests on main since #136, and would make backup_database refuse
after an operator compacts and restarts. Also escape two raw NUL bytes in
curve_store.rs that made git treat the file as binary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 63ef1f0f-89c1-4f06-9815-594ae3629bf3
📥 Commits

Reviewing files that changed from the base of the PR and between d32d8fc and 9b90f22.

⛔ Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 3
  • Cargo.toml
  • crates/convex-ext-redb/Cargo.toml
  • crates/convex-ext-redb/src/curve_store.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The workspace and convex-ext-redb manifests update the redb dependency requirement from version 4 to 4.4.

Changes: redb Dependency

Layer / File(s) Summary
Update redb requirement
Cargo.toml, crates/convex-ext-redb/Cargo.toml
The workspace dependency and crate requirement change from version 4 to 4.4.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9b90f

The dependency upgrade and related source changes are ready to merge based on the supplied verification.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the redb 4.4 upgrade and its purpose: fixing integrity failures after compaction.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sujitn
sujitn merged commit 2ba5351 into main Oct 22, 2026
15 checks passed
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