fix: leave MVStore's chunk retention and versions-to-keep at H2's defaults

[anidotnet]

Supersedes #1301 by @brettwooldridge, rebased onto main with one added commit.

MVStoreUtils.openOrCreate forced setRetentionTime(0) and setVersionsToKeep(0) on every store since 2020. With both at 0, H2 may reuse a chunk's blocks while the chunk map it writes at close still lists that chunk, and the file then refuses to open, read-only or not, with

MVStoreException: Double mark: 394/5 [...] at FreeSpaceBitSet.markUsed
  at RandomAccessStore.readStoreHeader

(h2database/h2database#2752, h2database/h2database#4083, both open). Only H2's recovery mode gets past it. A 24-thread soak of a document workload with a close every 20 seconds reproduced it on every run at 0/0, with and without close-time compaction, on h2-mvstore 2.4.240 and on current H2 master, and on none of the runs with either setting at its H2 default (45 s / 5), including a 1 s retention window.

MVStoreModuleBuilder gains retentionTime(ms) and versionsToKeep(n); both are null by default, which leaves H2's values in place. Passing 0 restores the old behaviour.

Added on top of #1301: MVStoreUtilsTest still pinned getVersionsToKeep() == 0, the value openOrCreate used to force, and failed. It now asserts H2's default of 5. getRetentionTime() stays 0 there because an in-memory store has no chunks to retain and H2 sets that itself.

This is also the corruption behind the intermittent MigrationTest.cleanUp failure on CI (AssertionError: chunk:5,block:0,len:0 ... at Chunk.accountForRemovedPage).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added configuration options for MVStore retention time and the number of versions to keep.
  • Unconfigured stores now preserve H2’s default retention settings.
  • Explicit retention settings are applied when provided.

• Bug Fixes

  • Corrected configuration cloning to retain the new MVStore options.

• Tests

  • Added coverage for default behavior, custom settings, and configuration cloning.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: c81cc799-bad6-4a9d-964a-5c2f738bf40f

📥 Commits

Reviewing files that changed from the base of the PR and between 23cb854 and dcdee1e.

📒 Files selected for processing (5)

• nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java
• nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java
• nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java
• nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreRetentionDefaultsTest.java
• nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreUtilsTest.java

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

---

📝 Walkthrough

Walkthrough

The MVStore adapter now exposes nullable retention settings. The builder propagates these settings to MVStoreConfig, and MVStoreUtils applies them only when configured. Otherwise, H2 retains its default values. Tests cover defaults, explicit settings, and configuration cloning.

Changes

MVStore retention configuration

Layer / File(s)	Summary
Retention configuration contract nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java, nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java	MVStoreConfig and MVStoreModuleBuilder now store nullable retentionTime and versionsToKeep values. build() transfers the values, and clone() preserves them.
Conditional MVStore application nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java	openOrCreate applies retention settings only when they are non-null. Unconfigured stores retain H2 defaults.
Retention behavior validation nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreRetentionDefaultsTest.java, nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreUtilsTest.java	Tests verify H2 defaults of 45,000 milliseconds and 5 versions, explicit settings, cloned options, and updated existing expectations.

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

Merge Risk: ⚪ Minimal · up to 0f618

MVStore now preserves H2 retention defaults unless values are explicitly configured, while retaining support for the prior zero-value behavior. The changed configuration flow and expected defaults are covered without an identified merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant MVStoreModuleBuilder
  participant MVStoreConfig
  participant MVStoreUtils
  participant MVStore
  MVStoreModuleBuilder->>MVStoreConfig: Build retentionTime and versionsToKeep
  MVStoreUtils->>MVStore: Apply non-null retention values
  MVStoreUtils->>MVStore: Preserve H2 defaults for null values

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 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 describes the main change: preserving MVStore chunk retention and versions-to-keep at H2 defaults.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/mvstore-retention-defaults

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[anidotnet]

Rebased onto main now that #1295 has landed.

The Ubuntu build failed on MigrationTest.testRepositoryMigrate with UniqueConstraintException: Unique key constraint violation for [empId], thrown from buildIndex inside ChangeIdField. That is the bug #1295 fixes, not a regression from this change: dropIndexDescriptor used to act only on the map name recorded in IndexMeta, so re-creating the index found the previous layout map still populated and re-added ids it already held. It surfaced here because leaving retention at H2's default stops the store panicking at close, which is what the failure used to be.

Verified locally on the rebase: MigrationTest, MVStoreUtilsTest and MVStoreRetentionDefaultsTest all pass.