Skip to content

Commit 8ed3f64

Browse files
anidotnetbrettwooldridgeclaude
authored
fix: leave MVStore's chunk retention and versions-to-keep at H2's defaults (#1303)
* 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 <noreply@anthropic.com> * Update MVStoreUtilsTest to H2's default versionsToKeep The test pinned the values openOrCreate used to force. versionsToKeep is now H2's own 5; retentionTime stays 0 because an in-memory store has no chunks to retain and H2 sets that itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Brett Wooldridge <brett.wooldridge@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 72264ec commit 8ed3f64

5 files changed

Lines changed: 152 additions & 4 deletions

File tree

‎nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreConfig.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ public class MVStoreConfig implements StoreConfig {
121121
*/
122122
private FileStore<?> fileStore;
123123

124+
/**
125+
* Chunk retention time in milliseconds; {@code null} leaves H2's default.
126+
*/
127+
@Setter(AccessLevel.PACKAGE)
128+
private Integer retentionTime;
129+
130+
/**
131+
* Old versions to keep; {@code null} leaves H2's default.
132+
*/
133+
@Setter(AccessLevel.PACKAGE)
134+
private Integer versionsToKeep;
135+
124136
MVStoreConfig() {
125137
eventListeners = new HashSet<>();
126138
}
@@ -150,6 +162,8 @@ public MVStoreConfig clone() {
150162
config.cacheSize(cacheSize);
151163
config.cacheConcurrency(cacheConcurrency);
152164
config.pageSplitSize(pageSplitSize);
165+
config.retentionTime(retentionTime);
166+
config.versionsToKeep(versionsToKeep);
153167
config.fileStore(fileStore);
154168
return config;
155169
}

‎nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,23 @@ public class MVStoreModuleBuilder {
122122
*/
123123
private FileStore<?> fileStore;
124124

125+
/**
126+
* How long MVStore keeps a chunk after its last live page is gone, in milliseconds, before
127+
* its blocks may be reused. {@code null} (the default) leaves H2's own default of 45 seconds.
128+
* <p>
129+
* Earlier releases forced this to 0 together with {@link #versionsToKeep}. With both at 0 a
130+
* chunk's blocks can be reused while the chunk map written at close still lists that chunk,
131+
* and the file then fails to open with "Double mark". Setting 0 restores that behaviour and
132+
* is not recommended.
133+
*/
134+
private Integer retentionTime;
135+
136+
/**
137+
* How many old versions MVStore keeps. {@code null} (the default) leaves H2's own default
138+
* of 5. See {@link #retentionTime}.
139+
*/
140+
private Integer versionsToKeep;
141+
125142
/**
126143
* The configuration for the MVStore.
127144
*/
@@ -193,6 +210,8 @@ public MVStoreModule build() {
193210
dbConfig.cacheSize(cacheSize());
194211
dbConfig.cacheConcurrency(cacheConcurrency());
195212
dbConfig.pageSplitSize(pageSplitSize());
213+
dbConfig.retentionTime(retentionTime());
214+
dbConfig.versionsToKeep(versionsToKeep());
196215
dbConfig.fileStore(fileStore());
197216
dbConfig.eventListeners(eventListeners());
198217

‎nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,17 @@ static MVStore openOrCreate(MVStoreConfig storeConfig) {
101101
throw new NitriteIOException("Unable to create database file", iae);
102102
} finally {
103103
if (store != null) {
104-
store.setRetentionTime(0);
105-
store.setVersionsToKeep(0);
104+
// H2's defaults (45 s retention, 5 versions) stay unless configured. Forcing both to 0,
105+
// as earlier releases did, let a chunk's blocks be reused while the chunk map written
106+
// at close still listed the chunk; the file then failed to open with "Double mark".
107+
Integer retentionTime = storeConfig.retentionTime();
108+
if (retentionTime != null) {
109+
store.setRetentionTime(retentionTime);
110+
}
111+
Integer versionsToKeep = storeConfig.versionsToKeep();
112+
if (versionsToKeep != null) {
113+
store.setVersionsToKeep(versionsToKeep);
114+
}
106115
}
107116
}
108117

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright (c) 2017-2020. Nitrite author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.dizitart.no2.mvstore;
18+
19+
import org.dizitart.no2.Nitrite;
20+
import org.h2.mvstore.MVStore;
21+
import org.junit.After;
22+
import org.junit.Before;
23+
import org.junit.Test;
24+
25+
import java.io.File;
26+
import java.nio.file.Files;
27+
import java.nio.file.Path;
28+
29+
import static org.junit.Assert.assertEquals;
30+
import static org.junit.Assert.assertNull;
31+
32+
/**
33+
* The adapter must not force MVStore's chunk retention time and versions-to-keep to 0. With both
34+
* at 0, H2 may reuse a chunk's blocks while the chunk map it writes at close still lists that
35+
* chunk, and the file then refuses to open with "Double mark" (h2database/h2database#2752,
36+
* #4083). A soak of a 24-thread workload with a close every 20 seconds hit that on every run at
37+
* 0/0 and on none with either of H2's defaults in place.
38+
*/
39+
public class MVStoreRetentionDefaultsTest {
40+
private Path directory;
41+
private Nitrite db;
42+
43+
@Before
44+
public void setUp() throws Exception {
45+
directory = Files.createTempDirectory("nitrite-retention");
46+
}
47+
48+
@After
49+
public void tearDown() throws Exception {
50+
if (db != null && !db.isClosed()) {
51+
db.close();
52+
}
53+
try (var files = Files.walk(directory)) {
54+
files.sorted((a, b) -> b.compareTo(a)).map(Path::toFile).forEach(File::delete);
55+
}
56+
}
57+
58+
@Test
59+
public void testDefaultsAreLeftToH2() {
60+
MVStoreModuleBuilder builder = MVStoreModule.withConfig();
61+
assertNull(builder.retentionTime());
62+
assertNull(builder.versionsToKeep());
63+
64+
db = Nitrite.builder().loadModule(builder.filePath(directory.resolve("test.db").toFile()).build()).openOrCreate();
65+
MVStore store = mvStore(db);
66+
assertEquals(45_000, store.getRetentionTime());
67+
assertEquals(5, store.getVersionsToKeep());
68+
}
69+
70+
@Test
71+
public void testConfiguredValuesAreApplied() {
72+
MVStoreModule module = MVStoreModule.withConfig()
73+
.filePath(directory.resolve("test.db").toFile())
74+
.retentionTime(1_000)
75+
.versionsToKeep(2)
76+
.build();
77+
db = Nitrite.builder().loadModule(module).openOrCreate();
78+
MVStore store = mvStore(db);
79+
assertEquals(1_000, store.getRetentionTime());
80+
assertEquals(2, store.getVersionsToKeep());
81+
}
82+
83+
@Test
84+
public void testConfigCloneCarriesTheOptions() {
85+
MVStoreConfig config = new MVStoreConfig();
86+
config.retentionTime(7);
87+
config.versionsToKeep(3);
88+
MVStoreConfig clone = config.clone();
89+
assertEquals(Integer.valueOf(7), clone.retentionTime());
90+
assertEquals(Integer.valueOf(3), clone.versionsToKeep());
91+
}
92+
93+
private static MVStore mvStore(Nitrite db) {
94+
try {
95+
var field = NitriteMVStore.class.getDeclaredField("mvStore");
96+
field.setAccessible(true);
97+
return (MVStore) field.get(db.getStore());
98+
} catch (ReflectiveOperationException e) {
99+
throw new AssertionError(e);
100+
}
101+
}
102+
}

‎nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreUtilsTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@ public class MVStoreUtilsTest {
2929
public void testOpenOrCreate() {
3030
MVStore actualOpenOrCreateResult = MVStoreUtils.openOrCreate(new MVStoreConfig());
3131
assertFalse(actualOpenOrCreateResult.isReadOnly());
32-
assertEquals(0L, actualOpenOrCreateResult.getVersionsToKeep());
32+
// H2's own default, no longer forced to 0
33+
assertEquals(5L, actualOpenOrCreateResult.getVersionsToKeep());
3334
assertEquals(0, actualOpenOrCreateResult.getUnsavedMemory());
3435
assertEquals(0, actualOpenOrCreateResult.getStoreVersion());
36+
// an in-memory store has no chunks to retain, so H2 keeps this at 0 by itself
3537
assertEquals(0, actualOpenOrCreateResult.getRetentionTime());
3638
assertEquals(0, actualOpenOrCreateResult.getMetaMap().size());
3739
assertEquals(Long.MAX_VALUE, actualOpenOrCreateResult.getMaxPageSize());
@@ -46,9 +48,11 @@ public void testOpenOrCreate2() {
4648
mvStoreConfig.addStoreEventListener(mock(StoreEventListener.class));
4749
MVStore actualOpenOrCreateResult = MVStoreUtils.openOrCreate(mvStoreConfig);
4850
assertFalse(actualOpenOrCreateResult.isReadOnly());
49-
assertEquals(0L, actualOpenOrCreateResult.getVersionsToKeep());
51+
// H2's own default, no longer forced to 0
52+
assertEquals(5L, actualOpenOrCreateResult.getVersionsToKeep());
5053
assertEquals(0, actualOpenOrCreateResult.getUnsavedMemory());
5154
assertEquals(0, actualOpenOrCreateResult.getStoreVersion());
55+
// an in-memory store has no chunks to retain, so H2 keeps this at 0 by itself
5256
assertEquals(0, actualOpenOrCreateResult.getRetentionTime());
5357
assertEquals(0, actualOpenOrCreateResult.getMetaMap().size());
5458
assertEquals(Long.MAX_VALUE, actualOpenOrCreateResult.getMaxPageSize());

0 commit comments

Comments
 (0)