Skip to content

fix: default MVStore pageSplitSize to 16 KB, not 16 bytes - #1293

Open
brettwooldridge wants to merge 1 commit into
nitrite:mainfrom
brettwooldridge:fix/mvstore-page-split-size-default
Open

fix: default MVStore pageSplitSize to 16 KB, not 16 bytes#1293
brettwooldridge wants to merge 1 commit into
nitrite:mainfrom
brettwooldridge:fix/mvstore-page-split-size-default

Conversation

@brettwooldridge

@brettwooldridge brettwooldridge commented Sep 3, 2026

Copy link
Copy Markdown

MVStoreModuleBuilder.pageSplitSize is documented as "16 KB" and is passed straight to MVStore.Builder.pageSplitSize, which takes bytes. Its value was 16, the same literal used for cacheSize (megabytes) and cacheConcurrency (a count), so every leaf page split as soon as it held more than one entry.

Measured by rebuilding a 146 MB store of 46,926 entries at each setting: 93,781 pages at depth 13 with 16; 36,096 pages at depth 12 with MVStore's own persistent-store default of 16 KB; 13,327 pages at depth 6 with 64 KB, the largest value that survives H2's (cacheSize / cacheConcurrency) >> 4 clamp.

The default is now 16 * 1024, matching the javadoc and MVStore's default. Existing files are unaffected until their pages are rewritten; there is nothing to migrate. The two tests that pinned the old literal through Short.SIZE are updated.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected the default MVStore page split size from 16 bytes to 16 KB.
    • Reduced excessive page splitting when using the default configuration.
  • Documentation

    • Updated configuration documentation to accurately describe the default page split size.

MVStoreModuleBuilder.pageSplitSize is documented as "16 KB" and is passed
straight to MVStore.Builder.pageSplitSize, which takes bytes. Its value was
16, the same literal used for cacheSize (megabytes) and cacheConcurrency (a
count), so every leaf page split as soon as it held more than one entry.

Measured by rebuilding a 146 MB store of 46,926 entries at each setting:
93,781 pages at depth 13 with 16; 36,096 pages at depth 12 with MVStore's
own persistent-store default of 16 KB; 13,327 pages at depth 6 with 64 KB,
the largest value that survives H2's (cacheSize / cacheConcurrency) >> 4
clamp.

The default is now 16 * 1024, matching the javadoc and MVStore's default.
Existing files are unaffected until their pages are rewritten; there is
nothing to migrate. The two tests that pinned the old literal through
Short.SIZE are updated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 8a49ea8c-19a3-48ae-85aa-2b3e30dedfde

📥 Commits

Reviewing files that changed from the base of the PR and between 38caf34 and 187537a.

📒 Files selected for processing (3)
  • nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java
  • nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreModuleBuilderTest.java
  • nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreModuleTest.java

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The MVStore module default page split size changed from 16 bytes to 16 KB. Documentation and constructor and configuration tests now use the updated default.

Changes

MVStore page split default

Layer / File(s) Summary
Update page split default and validation
nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java, nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreModuleBuilderTest.java, nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreModuleTest.java
The builder now uses and documents a 16 KB default page split size. Constructor and configuration tests assert 16 * 1024.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 18753

This corrects the default MVStore page split size to 16 KB and aligns its tests and documentation. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely states the main change: correcting the default MVStore pageSplitSize from 16 bytes to 16 KB.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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