From abecfd264bb2ce4b7b3246155092bd4a5729a12e Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Fri, 4 Sep 2026 19:42:46 +0900 Subject: [PATCH] fix: leave MVStore's chunk retention and versions-to-keep at H2's defaults 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, #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. H2's own javadoc notes the retention window is what lets readers finish traversing a map. 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 for anyone who depends on the file shrinking immediately, at the cost described above. Co-Authored-By: Claude Fable 5.1 --- .../dizitart/no2/mvstore/MVStoreConfig.java | 14 +++ .../no2/mvstore/MVStoreModuleBuilder.java | 19 ++++ .../dizitart/no2/mvstore/MVStoreUtils.java | 13 ++- .../mvstore/MVStoreRetentionDefaultsTest.java | 102 ++++++++++++++++++ 4 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreRetentionDefaultsTest.java diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java index cbfc94d9a..9bdf4f0bf 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java @@ -121,6 +121,18 @@ public class MVStoreConfig implements StoreConfig { */ private FileStore fileStore; + /** + * Chunk retention time in milliseconds; {@code null} leaves H2's default. + */ + @Setter(AccessLevel.PACKAGE) + private Integer retentionTime; + + /** + * Old versions to keep; {@code null} leaves H2's default. + */ + @Setter(AccessLevel.PACKAGE) + private Integer versionsToKeep; + MVStoreConfig() { eventListeners = new HashSet<>(); } @@ -150,6 +162,8 @@ public MVStoreConfig clone() { config.cacheSize(cacheSize); config.cacheConcurrency(cacheConcurrency); config.pageSplitSize(pageSplitSize); + config.retentionTime(retentionTime); + config.versionsToKeep(versionsToKeep); config.fileStore(fileStore); return config; } diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java index 63bcdddc5..cb9c6bce0 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java @@ -116,6 +116,23 @@ public class MVStoreModuleBuilder { */ private FileStore fileStore; + /** + * How long MVStore keeps a chunk after its last live page is gone, in milliseconds, before + * its blocks may be reused. {@code null} (the default) leaves H2's own default of 45 seconds. + *

+ * Earlier releases forced this to 0 together with {@link #versionsToKeep}. With both at 0 a + * chunk's blocks can be reused while the chunk map written at close still lists that chunk, + * and the file then fails to open with "Double mark". Setting 0 restores that behaviour and + * is not recommended. + */ + private Integer retentionTime; + + /** + * How many old versions MVStore keeps. {@code null} (the default) leaves H2's own default + * of 5. See {@link #retentionTime}. + */ + private Integer versionsToKeep; + /** * The configuration for the MVStore. */ @@ -187,6 +204,8 @@ public MVStoreModule build() { dbConfig.cacheSize(cacheSize()); dbConfig.cacheConcurrency(cacheConcurrency()); dbConfig.pageSplitSize(pageSplitSize()); + dbConfig.retentionTime(retentionTime()); + dbConfig.versionsToKeep(versionsToKeep()); dbConfig.fileStore(fileStore()); dbConfig.eventListeners(eventListeners()); diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java index e816ddd2e..026d823a4 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java @@ -101,8 +101,17 @@ static MVStore openOrCreate(MVStoreConfig storeConfig) { throw new NitriteIOException("Unable to create database file", iae); } finally { if (store != null) { - store.setRetentionTime(0); - store.setVersionsToKeep(0); + // H2's defaults (45 s retention, 5 versions) stay unless configured. Forcing both to 0, + // as earlier releases did, let a chunk's blocks be reused while the chunk map written + // at close still listed the chunk; the file then failed to open with "Double mark". + Integer retentionTime = storeConfig.retentionTime(); + if (retentionTime != null) { + store.setRetentionTime(retentionTime); + } + Integer versionsToKeep = storeConfig.versionsToKeep(); + if (versionsToKeep != null) { + store.setVersionsToKeep(versionsToKeep); + } } } diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreRetentionDefaultsTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreRetentionDefaultsTest.java new file mode 100644 index 000000000..4c7c9179e --- /dev/null +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreRetentionDefaultsTest.java @@ -0,0 +1,102 @@ +/* + * Copyright (c) 2017-2020. Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.dizitart.no2.mvstore; + +import org.dizitart.no2.Nitrite; +import org.h2.mvstore.MVStore; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +/** + * The adapter must not force MVStore's chunk retention time and versions-to-keep to 0. 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 with "Double mark" (h2database/h2database#2752, + * #4083). A soak of a 24-thread workload with a close every 20 seconds hit that on every run at + * 0/0 and on none with either of H2's defaults in place. + */ +public class MVStoreRetentionDefaultsTest { + private Path directory; + private Nitrite db; + + @Before + public void setUp() throws Exception { + directory = Files.createTempDirectory("nitrite-retention"); + } + + @After + public void tearDown() throws Exception { + if (db != null && !db.isClosed()) { + db.close(); + } + try (var files = Files.walk(directory)) { + files.sorted((a, b) -> b.compareTo(a)).map(Path::toFile).forEach(File::delete); + } + } + + @Test + public void testDefaultsAreLeftToH2() { + MVStoreModuleBuilder builder = MVStoreModule.withConfig(); + assertNull(builder.retentionTime()); + assertNull(builder.versionsToKeep()); + + db = Nitrite.builder().loadModule(builder.filePath(directory.resolve("test.db").toFile()).build()).openOrCreate(); + MVStore store = mvStore(db); + assertEquals(45_000, store.getRetentionTime()); + assertEquals(5, store.getVersionsToKeep()); + } + + @Test + public void testConfiguredValuesAreApplied() { + MVStoreModule module = MVStoreModule.withConfig() + .filePath(directory.resolve("test.db").toFile()) + .retentionTime(1_000) + .versionsToKeep(2) + .build(); + db = Nitrite.builder().loadModule(module).openOrCreate(); + MVStore store = mvStore(db); + assertEquals(1_000, store.getRetentionTime()); + assertEquals(2, store.getVersionsToKeep()); + } + + @Test + public void testConfigCloneCarriesTheOptions() { + MVStoreConfig config = new MVStoreConfig(); + config.retentionTime(7); + config.versionsToKeep(3); + MVStoreConfig clone = config.clone(); + assertEquals(Integer.valueOf(7), clone.retentionTime()); + assertEquals(Integer.valueOf(3), clone.versionsToKeep()); + } + + private static MVStore mvStore(Nitrite db) { + try { + var field = NitriteMVStore.class.getDeclaredField("mvStore"); + field.setAccessible(true); + return (MVStore) field.get(db.getStore()); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } +}