Skip to content

Commit d0e03eb

Browse files
committed
fix: Massive insertion on cluster config issues
Fixed Issue #4219
1 parent 748e81b commit d0e03eb

3 files changed

Lines changed: 181 additions & 2 deletions

File tree

engine/src/main/java/com/arcadedb/engine/LocalBucket.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ public LocalBucket(final DatabaseInternal database, final String name, final Str
164164

165165
/**
166166
* Called at load time.
167+
* <p>
168+
* Free-space statistics are NOT pre-warmed here. {@link #findAvailableSpace} already calls
169+
* {@link #gatherPageStatistics()} lazily on the first allocation that needs it, so a leader
170+
* still gets reuse for free; a follower that only applies leader-shipped pages via the state
171+
* machine never triggers it at all. Pre-warming here would scan up to all pages of every
172+
* bucket during {@link com.arcadedb.schema.LocalSchema#load} - which on a follower under
173+
* heavy bulk-load fires repeatedly per LSM compaction SCHEMA_ENTRY and exhausts the heap
174+
* (issue #4219).
167175
*/
168176
public LocalBucket(final DatabaseInternal database, final String name, final String filePath, final int id,
169177
final ComponentFile.MODE mode, final int pageSize, final int version) throws IOException {
@@ -178,8 +186,6 @@ public LocalBucket(final DatabaseInternal database, final String name, final Str
178186
// by default and bypassed the user-DML guard. Schema JSON still maps primary->external by name (which is
179187
// an orthogonal concern), but the write guard now no longer depends on schema-load ordering.
180188
this.purpose = purposeForVersion(version);
181-
if (this.reuseSpaceMode.ordinal() >= REUSE_SPACE_MODE.HIGH.ordinal())
182-
gatherPageStatistics();
183189
}
184190

185191
private static Purpose purposeForVersion(final int version) {
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* Copyright © 2021-present Arcade Data Ltd (info@arcadedata.com)
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+
* SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com)
17+
* SPDX-License-Identifier: Apache-2.0
18+
*/
19+
package com.arcadedb.engine;
20+
21+
import com.arcadedb.GlobalConfiguration;
22+
import com.arcadedb.database.Database;
23+
import com.arcadedb.database.DatabaseFactory;
24+
import com.arcadedb.database.DatabaseInternal;
25+
import com.arcadedb.schema.LocalSchema;
26+
import com.arcadedb.utility.FileUtils;
27+
import org.junit.jupiter.api.Test;
28+
29+
import java.io.File;
30+
31+
import static org.assertj.core.api.Assertions.assertThat;
32+
33+
/**
34+
* Regression test for issue #4219.
35+
* <p>
36+
* On a Raft follower under heavy bulk insertion, every LSM compaction on the leader fires a
37+
* SCHEMA_ENTRY that the follower applies via {@code LocalSchema.load(MODE.READ_WRITE, true)}.
38+
* Before the fix, that load() path destroyed every in-memory bucket/index instance and the
39+
* load-time {@link LocalBucket} constructor called {@code gatherPageStatistics()} eagerly.
40+
* For buckets whose pages were mostly full (typical under bulk insert) the scan walked every
41+
* single page of every bucket on every reload, pulling them all through the page cache and
42+
* the active TransactionContext. Across thousands of compaction events, this exhausted the
43+
* follower heap and surfaced as a Java heap space OOM in
44+
* {@code com.arcadedb.engine.PageManager.loadPage}, followed by cascading "Bucket with id X
45+
* was not found" SchemaExceptions on subsequent TX_ENTRY applies (the schema reload had
46+
* already cleared bucketMap before the OOM aborted it).
47+
* <p>
48+
* After the fix, the load-time bucket constructor no longer pre-warms statistics.
49+
* {@link LocalBucket#findAvailableSpace} already calls {@code gatherPageStatistics()} lazily
50+
* on the first allocation, so a leader still gets reuse behavior for free; a follower that
51+
* only applies leader-shipped page bytes via the state machine never triggers the scan at all.
52+
*
53+
* @author Luca Garulli (l.garulli@arcadedata.com)
54+
*/
55+
class SchemaReloadDoesNotEagerScanBucketsTest {
56+
57+
private static final String DB_PATH = "./target/databases/SchemaReloadDoesNotEagerScanBuckets";
58+
private static final String TYPE = "BulkRow";
59+
private static final int REC_COUNT = 5_000;
60+
private static final int PAYLOAD_KB = 1;
61+
62+
@Test
63+
void schemaReloadDoesNotEagerlyScanBucketPages() throws Exception {
64+
final String previousMode = GlobalConfiguration.BUCKET_REUSE_SPACE_MODE.getValueAsString();
65+
GlobalConfiguration.BUCKET_REUSE_SPACE_MODE.setValue("high");
66+
67+
FileUtils.deleteRecursively(new File(DB_PATH));
68+
try (final DatabaseFactory factory = new DatabaseFactory(DB_PATH)) {
69+
// Step 1: create the database and fill the bucket with many pages. Default page size is
70+
// ~64KB; 5_000 records with a 1 KB payload span dozens of pages, enough to make a full
71+
// bucket-page scan during schema reload measurable.
72+
try (final Database db = factory.create()) {
73+
db.getSchema().createDocumentType(TYPE, 1);
74+
final String payload = "x".repeat(PAYLOAD_KB * 1024);
75+
db.transaction(() -> {
76+
for (int i = 0; i < REC_COUNT; i++)
77+
db.newDocument(TYPE).set("idx", i).set("payload", payload).save();
78+
});
79+
}
80+
81+
try (final Database db = factory.open()) {
82+
final DatabaseInternal internal = (DatabaseInternal) db;
83+
final PageManager pageManager = internal.getPageManager();
84+
85+
// The open path warmed schema state. Evict everything we cached for this database so the
86+
// next reload has to go to disk - the same situation a Raft follower hits during a
87+
// SCHEMA_ENTRY apply after a long bulk-load run that has churned the page cache.
88+
pageManager.removeAllReadPagesOfDatabase(db);
89+
90+
final long pagesReadBaseline = pageManager.getStats().pagesRead;
91+
92+
// Simulate the follower SCHEMA_ENTRY path: full schema reload from disk.
93+
((LocalSchema) internal.getSchema().getEmbedded()).load(ComponentFile.MODE.READ_WRITE, true);
94+
95+
final long pagesReadDelta = pageManager.getStats().pagesRead - pagesReadBaseline;
96+
97+
// Before the fix, gatherPageStatistics() ran from the bucket constructor on every load
98+
// and read every full page (often hundreds for a 5_000-record bucket). After the fix,
99+
// schema reload touches only the dictionary, schema.json, and bucket headers - a small,
100+
// fixed amount of I/O independent of bucket size.
101+
assertThat(pagesReadDelta)
102+
.as("Schema reload must not eagerly scan bucket data pages (issue #4219)")
103+
.isLessThan(30L);
104+
}
105+
} finally {
106+
GlobalConfiguration.BUCKET_REUSE_SPACE_MODE.setValue(previousMode);
107+
FileUtils.deleteRecursively(new File(DB_PATH));
108+
}
109+
}
110+
111+
@Test
112+
void lazyPageStatisticsStillWorkAfterReload() throws Exception {
113+
// Sanity check that removing the eager warmup did not regress the lazy path: in HIGH mode,
114+
// the first allocation after a reload must still trigger gatherPageStatistics() (so writes
115+
// continue to recycle freed space on a leader). Measured indirectly via pagesRead delta.
116+
final String previousMode = GlobalConfiguration.BUCKET_REUSE_SPACE_MODE.getValueAsString();
117+
GlobalConfiguration.BUCKET_REUSE_SPACE_MODE.setValue("high");
118+
119+
FileUtils.deleteRecursively(new File(DB_PATH));
120+
try (final DatabaseFactory factory = new DatabaseFactory(DB_PATH)) {
121+
try (final Database db = factory.create()) {
122+
db.getSchema().createDocumentType(TYPE, 1);
123+
final String payload = "y".repeat(PAYLOAD_KB * 1024);
124+
db.transaction(() -> {
125+
for (int i = 0; i < REC_COUNT; i++)
126+
db.newDocument(TYPE).set("idx", i).set("payload", payload).save();
127+
});
128+
}
129+
130+
try (final Database db = factory.open()) {
131+
final DatabaseInternal internal = (DatabaseInternal) db;
132+
final PageManager pageManager = internal.getPageManager();
133+
134+
// Force a fresh schema reload and drop cached pages so the lazy scan we want to trigger
135+
// below has to read from disk (which is what bumps pagesRead).
136+
((LocalSchema) internal.getSchema().getEmbedded()).load(ComponentFile.MODE.READ_WRITE, true);
137+
pageManager.removeAllReadPagesOfDatabase(db);
138+
139+
final long pagesReadBeforeAlloc = pageManager.getStats().pagesRead;
140+
141+
// First write after the reload: the allocator must run gatherPageStatistics lazily,
142+
// which scans pages and increases the page-read counter (proving the lazy path fires).
143+
db.transaction(() -> db.newDocument(TYPE).set("idx", -1).set("payload", "trigger").save());
144+
145+
final long pagesReadAfterAlloc = pageManager.getStats().pagesRead;
146+
assertThat(pagesReadAfterAlloc - pagesReadBeforeAlloc)
147+
.as("First allocation after a reload must trigger lazy gatherPageStatistics")
148+
.isGreaterThan(0L);
149+
}
150+
} finally {
151+
GlobalConfiguration.BUCKET_REUSE_SPACE_MODE.setValue(previousMode);
152+
FileUtils.deleteRecursively(new File(DB_PATH));
153+
}
154+
}
155+
}

ha-raft/src/main/java/com/arcadedb/server/ha/raft/ArcadeStateMachine.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ public record BootstrapBaseline(String fingerprint, long lastTxId) {
126126

127127
private final AtomicBoolean needsSnapshotDownload = new AtomicBoolean(false);
128128
private final AtomicBoolean catchingUp = new AtomicBoolean(false);
129+
// Set to true after applyTransaction catches an unexpected Throwable (OOM, NPE, etc.). The
130+
// state machine's in-memory schema/page state can be inconsistent at that point (issue #4219:
131+
// mid-load OOM leaves bucketMap cleared but not repopulated), so any subsequent apply
132+
// attempt would surface as a cascade of "Bucket with id X was not found" errors before the
133+
// async server.stop() completes. Once tripped, applyTransaction fails fast without touching
134+
// database state and the recovery path is the asynchronous server shutdown plus a snapshot
135+
// resync on the next start.
136+
private final AtomicBoolean haltedAfterCriticalError = new AtomicBoolean(false);
129137

130138

131139
public void setServer(final ArcadeDBServer server) {
@@ -235,6 +243,13 @@ public CompletableFuture<Message> applyTransaction(final TransactionContext trx)
235243
final TermIndex termIndex = TermIndex.valueOf(entry);
236244
final long index = termIndex.getIndex();
237245

246+
// Refuse to apply once a prior entry tripped the critical-error halt. Continuing would
247+
// operate on the inconsistent in-memory state left behind by the failed apply and cascade
248+
// into additional SEVERE errors before the async server.stop() completes (#4219).
249+
if (haltedAfterCriticalError.get())
250+
return CompletableFuture.failedFuture(new ReplicationException(
251+
"State machine halted after critical error at earlier index; refusing to apply index " + index));
252+
238253
try {
239254
final RaftLogEntryCodec.DecodedEntry decoded = RaftLogEntryCodec.decode(data);
240255

@@ -299,6 +314,9 @@ public CompletableFuture<Message> applyTransaction(final TransactionContext trx)
299314
"""
300315
CRITICAL: Unexpected error applying Raft log entry at index %d. \
301316
Shutting down to prevent state divergence.""", e, index);
317+
// Trip the halt flag BEFORE starting the async server.stop() so the StateMachineUpdater's
318+
// next applyTransaction call short-circuits instead of cascading on inconsistent state.
319+
haltedAfterCriticalError.set(true);
302320
final Thread stopThread = new Thread(() -> {
303321
try {
304322
if (server != null)

0 commit comments

Comments
 (0)