From 5b4ca377eeb6595c81a2d993af05d8746ef5a3ca Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:48:09 +0100 Subject: [PATCH 1/3] perf: write periodic station progress off the main thread The 30-second progress save fsynced stations.json on the server thread (about 25 s of main-thread fsync per day on Main). It now snapshots the stations on the main thread and writes them with the same temp file, fsync and atomic rename on a single station writer thread. Every state change (construction start, completion, cancellation, station removal, reload and disable) still saves synchronously and reports failure as before. Each snapshot carries a sequence number and a queued snapshot older than one already written is dropped, so a background write can never put back state older than a synchronous checkpoint. Disable saves synchronously, then drains and stops the writer. Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/vfbuilders/data/Database.java | 119 ++++++++++++++--- .../vfbuilders/managers/StationManager.java | 4 +- .../vfbuilders/DatabaseFixture.java | 4 +- .../tfminecraft/vfbuilders/SchedulerTest.java | 9 +- .../vfbuilders/data/StationWriterTest.java | 124 ++++++++++++++++++ 5 files changed, 239 insertions(+), 21 deletions(-) create mode 100644 src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java diff --git a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java index 43afe2b..5ac2c10 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java +++ b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java @@ -17,37 +17,124 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.StandardCopyOption; +import java.time.Duration; import java.util.*; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; public class Database { private static final File file = new File(VFBuilders.plugin.getDataFolder(), "data/stations.json"); private static final Gson gson = new GsonBuilder().setPrettyPrinting().create(); + /** Held while a snapshot file is written; package-visible so tests can hold the writer. */ + static final Object WRITE_LOCK = new Object(); + private static final AtomicLong snapshots = new AtomicLong(); + // Guarded by WRITE_LOCK: the newest snapshot that has started writing. + private static long newestWrite; + private static ExecutorService writer; + public static void saveStations(Map stations) { saveStationsNow(stations); } + /** + * Writes the stations durably before returning. Callers that must know the state is on disk + * (construction start, completion, cancellation, station removal, disable) use this. + */ public static boolean saveStationsNow(Map stations) { try { - List dataList = new ArrayList<>(); - - for (ActiveStation station : stations.values()) { - StationData data = new StationData(); - data.location = toSerializableLoc(station.getLocation()); - data.stationId = station.getStation().getId(); // Save station identifier - data.spawnLocation = station.hasSpawnLocation() ? toSerializableLoc(station.getSpawnLocation()) : null; - data.blueprintId = station.hasBlueprint() ? station.getBlueprint().getId() : null; - data.timeLeft = station.getTimeLeft(); - data.outputUuid = station.getOutputUuid(); - if (station.hasBlueprint()) { - data.constructorUuid = toText(station.getConstructorUuid()); - data.constructorCharacterId = toText(station.getConstructorCharacterId()); - } + return write(snapshot(stations)); + } catch (RuntimeException e) { + e.printStackTrace(); + return false; + } + } + + /** + * Snapshots the stations on the calling thread and writes them durably on the station writer + * thread. A snapshot taken earlier than one already written is dropped, so a later synchronous + * save is never overwritten by an older queued one. + */ + public static CompletableFuture saveStationsLater(Map stations) { + Snapshot snapshot; + try { + snapshot = snapshot(stations); + } catch (RuntimeException e) { + e.printStackTrace(); + return CompletableFuture.completedFuture(false); + } + return CompletableFuture.supplyAsync(() -> write(snapshot), writer()); + } + + /** Finishes queued writes and stops the writer thread; the next background save starts a new one. */ + public static void close() { + close(Duration.ofSeconds(30)); + } + + static void close(Duration timeout) { + ExecutorService current; + synchronized (Database.class) { + current = writer; + writer = null; + } + if (current == null) return; + current.shutdown(); + try { + if (!current.awaitTermination(timeout.toMillis(), TimeUnit.MILLISECONDS)) { + VFBuilders.plugin.getLogger().warning( + "Station writer still busy after " + timeout.toSeconds() + "s; the last synchronous save stands."); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } - dataList.add(data); + private static synchronized ExecutorService writer() { + if (writer == null) { + writer = Executors.newSingleThreadExecutor(task -> { + Thread thread = new Thread(task, "VFBuilders station writer"); + thread.setDaemon(true); + return thread; + }); + } + return writer; + } + + private static Snapshot snapshot(Map stations) { + List dataList = new ArrayList<>(); + + for (ActiveStation station : stations.values()) { + StationData data = new StationData(); + data.location = toSerializableLoc(station.getLocation()); + data.stationId = station.getStation().getId(); // Save station identifier + data.spawnLocation = station.hasSpawnLocation() ? toSerializableLoc(station.getSpawnLocation()) : null; + data.blueprintId = station.hasBlueprint() ? station.getBlueprint().getId() : null; + data.timeLeft = station.getTimeLeft(); + data.outputUuid = station.getOutputUuid(); + if (station.hasBlueprint()) { + data.constructorUuid = toText(station.getConstructorUuid()); + data.constructorCharacterId = toText(station.getConstructorCharacterId()); } + dataList.add(data); + } + return new Snapshot(snapshots.incrementAndGet(), dataList); + } + + private static boolean write(Snapshot snapshot) { + synchronized (WRITE_LOCK) { + if (snapshot.sequence() < newestWrite) return true; // A newer snapshot already wrote. + newestWrite = snapshot.sequence(); + return writeFile(snapshot.stations()); + } + } + + private static boolean writeFile(List dataList) { + try { var directory = file.toPath().getParent(); Files.createDirectories(directory); var temporary = Files.createTempFile(directory, "stations-", ".tmp"); @@ -70,6 +157,8 @@ public static boolean saveStationsNow(Map stations) { } } + private record Snapshot(long sequence, List stations) {} + public static HashMap loadStations() { HashMap loaded = new HashMap<>(); diff --git a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java index 6f1a0c9..775ca21 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java +++ b/src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java @@ -82,7 +82,8 @@ public void start() { @Override public void run() { if (stations.values().stream().anyMatch(ActiveStation::hasBlueprint)) { - Database.saveStationsNow(stations); + // Only countdown progress changes between the synchronous checkpoints. + Database.saveStationsLater(stations); } } }.runTaskTimer(VFBuilders.plugin, 600L, 600L); @@ -93,6 +94,7 @@ public void stop() { station.removeDisplay(); } Database.saveStations(stations); + Database.close(); } public void rebindAfterReload(Runnable reloadDefinitions) { diff --git a/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java b/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java index 4ebf571..b547941 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java +++ b/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java @@ -5,7 +5,7 @@ import java.nio.file.Path; /** One data directory per JVM, matching Database's static path lifetime. */ -final class DatabaseFixture { +public final class DatabaseFixture { private static final Path ROOT; static { @@ -16,7 +16,7 @@ final class DatabaseFixture { } } - static Path folder() throws IOException { + public static Path folder() throws IOException { return Files.createDirectories(ROOT); } } diff --git a/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java b/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java index d03af0d..7735ef9 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java @@ -75,15 +75,18 @@ void ticksPauseIncompleteWorkFinishReadyWorkAndRefreshOnlyLoadedChunks() throws verify(waiting).rebindDefinitions(); db.clearInvocations(); server.getScheduler().performTicks(579); - db.verify(() -> Database.saveStationsNow(map)); + db.verify(() -> Database.saveStationsLater(map)); + db.verify(() -> Database.saveStationsNow(map), never()); when(ready.hasBlueprint()).thenReturn(false); when(waiting.hasBlueprint()).thenReturn(false); when(detached.hasBlueprint()).thenReturn(false); db.clearInvocations(); server.getScheduler().performTicks(600); - db.verify(() -> Database.saveStationsNow(map), never()); + db.verify(() -> Database.saveStationsLater(map), never()); manager.stop(); - db.verify(() -> Database.saveStations(map)); + var order = inOrder(Database.class); + order.verify(db, () -> Database.saveStations(map)); + order.verify(db, Database::close); } finally { MockBukkit.unmock(); VFBuilders.plugin = null; diff --git a/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java new file mode 100644 index 0000000..f8a0216 --- /dev/null +++ b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java @@ -0,0 +1,124 @@ +package net.tfminecraft.vfbuilders.data; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.nio.file.*; +import java.time.Duration; +import java.util.*; +import java.util.concurrent.*; +import java.util.logging.Logger; +import net.tfminecraft.vfbuilders.DatabaseFixture; +import net.tfminecraft.vfbuilders.VFBuilders; +import net.tfminecraft.vfbuilders.core.*; +import org.bukkit.*; +import org.junit.jupiter.api.*; + +class StationWriterTest { + private Path file; + + @BeforeEach + void plugin() throws Exception { + Path folder = DatabaseFixture.folder(); + VFBuilders.plugin = mock(VFBuilders.class); + when(VFBuilders.plugin.getDataFolder()).thenReturn(folder.toFile()); + file = Files.createDirectories(folder.resolve("data")).resolve("stations.json"); + } + + @AfterEach + void cleanUp() throws Exception { + Database.close(); + VFBuilders.plugin = null; + Files.deleteIfExists(file); + } + + private static Map stations(int timeLeft) { + var world = mock(World.class); + when(world.getName()).thenReturn("build-world"); + var loc = new Location(world, 3, 64, 5); + var definition = mock(Station.class); + when(definition.getId()).thenReturn("dock"); + var station = mock(ActiveStation.class); + when(station.getLocation()).thenReturn(loc); + when(station.getStation()).thenReturn(definition); + when(station.getTimeLeft()).thenReturn(timeLeft); + return Map.of(loc, station); + } + + @Test + void backgroundSaveWritesTheSnapshotTakenOnTheCallingThread() throws Exception { + var stations = stations(41); + var station = stations.values().iterator().next(); + var saved = Database.saveStationsLater(stations); + // Later changes are not part of the queued snapshot. + when(station.getTimeLeft()).thenReturn(7); + assertTrue(saved.get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 41")); + try (var paths = Files.list(file.getParent())) { + assertEquals(List.of(file), paths.toList()); + } + } + + @Test + void queuedOlderSnapshotNeverOverwritesALaterSynchronousSave() throws Exception { + CompletableFuture older; + synchronized (Database.WRITE_LOCK) { + older = Database.saveStationsLater(stations(30)); + // The writer thread is now waiting for the lock; this synchronous save is newer. + assertTrue(Database.saveStationsNow(stations(12))); + } + assertTrue(older.get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 12")); + assertTrue(Database.saveStationsLater(stations(11)).get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 11")); + } + + @Test + void snapshotFailureIsReportedWithoutQueueingAWrite() throws Exception { + Files.writeString(file, "[]"); + var detached = mock(ActiveStation.class); + when(detached.getLocation()).thenReturn(new Location(null, 1, 2, 3)); + assertFalse(Database.saveStationsLater(Map.of(new Location(null, 1, 2, 3), detached)).get()); + assertEquals("[]", Files.readString(file)); + } + + @Test + void closeDrainsQueuedWritesAndTheWriterRestartsAfterwards() throws Exception { + Database.close(); // Nothing started yet. + CompletableFuture queued; + synchronized (Database.WRITE_LOCK) { + queued = Database.saveStationsLater(stations(9)); + } + Database.close(); + assertTrue(queued.isDone()); + assertTrue(queued.get()); + assertTrue(Files.readString(file).contains("\"timeLeft\": 9")); + assertTrue(Database.saveStationsLater(stations(8)).get(5, TimeUnit.SECONDS)); + assertTrue(Files.readString(file).contains("\"timeLeft\": 8")); + } + + @Test + void closeGivesUpWaitingForAStuckWriter() throws Exception { + var logger = mock(Logger.class); + when(VFBuilders.plugin.getLogger()).thenReturn(logger); + CompletableFuture stuck; + synchronized (Database.WRITE_LOCK) { + stuck = Database.saveStationsLater(stations(5)); + Database.close(Duration.ofMillis(50)); + verify(logger).warning(contains("Station writer still busy")); + } + assertTrue(stuck.get(5, TimeUnit.SECONDS)); + } + + @Test + void interruptedCloseKeepsTheInterruptFlag() throws Exception { + CompletableFuture pending; + synchronized (Database.WRITE_LOCK) { + pending = Database.saveStationsLater(stations(4)); + Thread.currentThread().interrupt(); + Database.close(); + assertTrue(Thread.interrupted()); + } + assertTrue(pending.get(5, TimeUnit.SECONDS)); + } +} From 9d9bd44405204cc977b15dea1ce620d06764a125 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:57:52 +0100 Subject: [PATCH 2/3] test: read the stations path from Database in StationWriterTest Database fixes its file when the class first loads. On CI another test loads it under its own temp folder first, so the writer tests read the fixture path and found nothing. They now ask Database for the real path. Co-Authored-By: Claude Opus 5.5 --- src/main/java/net/tfminecraft/vfbuilders/data/Database.java | 6 ++++++ .../net/tfminecraft/vfbuilders/data/StationWriterTest.java | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java index 5ac2c10..3865ed7 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java +++ b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java @@ -16,6 +16,7 @@ import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.time.Duration; import java.util.*; @@ -37,6 +38,11 @@ public class Database { private static long newestWrite; private static ExecutorService writer; + /** The stations file; fixed when this class first loads, which is why tests read it from here. */ + static Path path() { + return file.toPath(); + } + public static void saveStations(Map stations) { saveStationsNow(stations); } diff --git a/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java index f8a0216..83a27df 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java @@ -22,7 +22,9 @@ void plugin() throws Exception { Path folder = DatabaseFixture.folder(); VFBuilders.plugin = mock(VFBuilders.class); when(VFBuilders.plugin.getDataFolder()).thenReturn(folder.toFile()); - file = Files.createDirectories(folder.resolve("data")).resolve("stations.json"); + // Database fixes its path when the class first loads, possibly under another test's folder. + file = Database.path(); + Files.createDirectories(file.getParent()); } @AfterEach From 05db267d1f84f16e5c880dc67ef30ea038969df3 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:03:40 +0100 Subject: [PATCH 3/3] fix: resolve the stations file when it is used Database fixed its path from whichever plugin data folder was set when the class first loaded, so test classes broke each other depending on run order. The path now comes from the plugin's data folder at save and load time; the background writer gets it inside the snapshot taken on the calling thread. On the server the data folder never changes, so saves and loads use the same file as before. Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/vfbuilders/data/Database.java | 19 ++++++++++++------- .../vfbuilders/DatabaseFixture.java | 2 +- .../vfbuilders/data/StationWriterTest.java | 1 - 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java index 3865ed7..ad5e692 100644 --- a/src/main/java/net/tfminecraft/vfbuilders/data/Database.java +++ b/src/main/java/net/tfminecraft/vfbuilders/data/Database.java @@ -28,7 +28,6 @@ public class Database { - private static final File file = new File(VFBuilders.plugin.getDataFolder(), "data/stations.json"); private static final Gson gson = new GsonBuilder().setPrettyPrinting().create(); /** Held while a snapshot file is written; package-visible so tests can hold the writer. */ @@ -38,9 +37,13 @@ public class Database { private static long newestWrite; private static ExecutorService writer; - /** The stations file; fixed when this class first loads, which is why tests read it from here. */ + /** The stations file, under the plugin's current data folder. */ static Path path() { - return file.toPath(); + return file().toPath(); + } + + private static File file() { + return new File(VFBuilders.plugin.getDataFolder(), "data/stations.json"); } public static void saveStations(Map stations) { @@ -128,18 +131,18 @@ private static Snapshot snapshot(Map stations) { dataList.add(data); } - return new Snapshot(snapshots.incrementAndGet(), dataList); + return new Snapshot(snapshots.incrementAndGet(), file(), dataList); } private static boolean write(Snapshot snapshot) { synchronized (WRITE_LOCK) { if (snapshot.sequence() < newestWrite) return true; // A newer snapshot already wrote. newestWrite = snapshot.sequence(); - return writeFile(snapshot.stations()); + return writeFile(snapshot.target(), snapshot.stations()); } } - private static boolean writeFile(List dataList) { + private static boolean writeFile(File file, List dataList) { try { var directory = file.toPath().getParent(); Files.createDirectories(directory); @@ -163,11 +166,13 @@ private static boolean writeFile(List dataList) { } } - private record Snapshot(long sequence, List stations) {} + // The target is resolved on the calling thread, so the writer thread never reads plugin state. + private record Snapshot(long sequence, File target, List stations) {} public static HashMap loadStations() { HashMap loaded = new HashMap<>(); + File file = file(); if (!file.exists()) return loaded; try (Reader reader = new FileReader(file)) { diff --git a/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java b/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java index b547941..6f69291 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java +++ b/src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java @@ -4,7 +4,7 @@ import java.nio.file.Files; import java.nio.file.Path; -/** One data directory per JVM, matching Database's static path lifetime. */ +/** One shared data directory per JVM for tests that save stations. */ public final class DatabaseFixture { private static final Path ROOT; diff --git a/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java index 83a27df..62d47c1 100644 --- a/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java +++ b/src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java @@ -22,7 +22,6 @@ void plugin() throws Exception { Path folder = DatabaseFixture.folder(); VFBuilders.plugin = mock(VFBuilders.class); when(VFBuilders.plugin.getDataFolder()).thenReturn(folder.toFile()); - // Database fixes its path when the class first loads, possibly under another test's folder. file = Database.path(); Files.createDirectories(file.getParent()); }