From 1bcddcc14e4dd02e0a6bd14c7b8e1958a86992c7 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 23 Oct 2026 01:21:17 +0200 Subject: [PATCH] Save vehicle checkpoints in one transaction and stop idle switch updates The 5-minute vehicle checkpoint committed every vehicle separately on the main thread, waiting for the disk once per vehicle, then copied the whole database as a backup. On Main that froze the server for 0.3-0.6 s every 5 minutes. Save all vehicles in one transaction instead, and fall back to one-by-one saves if that transaction fails so a rollback loses nothing. The backup copy now runs on the first checkpoint and then every 6th (30 minutes); the WAL checkpoint still runs every cycle. Track switches were animated by reading the tags of every rail display on every tick and re-teleporting each switch even when it was already in place. Keep the switch displays and their junctions in their own list, and skip the teleport when the yaw would not change. Co-Authored-By: Claude Opus 5.5 --- .../database/VehiclePersistence.java | 32 ++++++++++++ .../managers/VehicleManager.java | 19 +++++-- .../tracks/TrackDisplayManager.java | 33 ++++++++---- .../database/DatabaseRuntimeCoverageTest.java | 50 +++++++++++++++++++ .../managers/VehicleManagerCoverageTest.java | 23 +++++++-- .../TrackDisplayManagerCoverageTest.java | 29 +++++++++++ 6 files changed, 169 insertions(+), 17 deletions(-) diff --git a/src/main/java/net/tfminecraft/vehicleframework/database/VehiclePersistence.java b/src/main/java/net/tfminecraft/vehicleframework/database/VehiclePersistence.java index 2e7d2e2..23d8ea0 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/database/VehiclePersistence.java +++ b/src/main/java/net/tfminecraft/vehicleframework/database/VehiclePersistence.java @@ -1,6 +1,8 @@ package net.tfminecraft.vehicleframework.database; import java.io.File; +import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -66,6 +68,36 @@ public VehiclePersistResult saveLiveResult(ActiveVehicle vehicle) { } } + /** + * Saves the vehicles in one transaction, so a checkpoint waits for one commit instead of one per vehicle. + * If the transaction itself fails, each vehicle is saved on its own as before, so a rollback loses nothing. + */ + public Map saveLiveBatch(Collection vehicles) { + Map results = new LinkedHashMap<>(); + if (vehicles == null || vehicles.isEmpty()) { + return results; + } + if (repository != null) { + try { + repository.runInTransaction(() -> { + for (ActiveVehicle vehicle : vehicles) { + results.put(vehicle, saveLiveResult(vehicle)); + } + }); + return results; + } catch (RuntimeException ex) { + String message = ex.getMessage(); + log("SQLite checkpoint transaction failed, saving vehicles one by one" + + (message == null || message.isBlank() ? "" : ": " + message)); + results.clear(); + } + } + for (ActiveVehicle vehicle : vehicles) { + results.put(vehicle, saveLiveResult(vehicle)); + } + return results; + } + public VehiclePersistResult resolveFailedLiveSave(String uuid, String reason) { String why = reason == null || reason.isBlank() ? "unknown" : reason; if (uuid != null && !uuid.isBlank() && repository != null) { diff --git a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java index e2d09dc..290e174 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java +++ b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java @@ -106,6 +106,9 @@ public class VehicleManager implements Listener{ public static final String TICKET_NEEDED = "§cYou need a ticket for this vehicle."; public static final String TICKET_PASSENGER_ONLY = "§cA ticket only lets you ride in a passenger seat."; public static final String NOT_ALLOWED_ABOARD = "§cThe owner has not allowed you aboard this vehicle."; + // Checkpoints run every 5 minutes; the SQLite backup copy runs on every 6th. + static final int BACKUP_EVERY_CHECKPOINTS = 6; + private int checkpoints; private ItemAPI api = TLibs.getItemAPI(); private InventoryManager inv = new InventoryManager(); @@ -352,18 +355,24 @@ public void run() { VFLogger.log("Checkpoint skipped: SQLite is not open"); return; } + List live = new ArrayList<>(); for (ActiveVehicle v : vehicles.values()) { - if (v.isDestroyed()) { - continue; + if (!v.isDestroyed()) { + live.add(v); } - VehiclePersistResult result = persistence.saveLiveResult(v); + } + persistence.saveLiveBatch(live).forEach((v, result) -> { if (result.isFailed()) { VFLogger.log("Failed to persist " + describeVehicle(v) + " at " + describeLocation(v) + " during checkpoint: " + result.reason()); } - } + }); persistence.checkpointWal(false); - persistence.vacuumIntoBackup(); + // The backup copies the whole database on the main thread, so it runs every + // BACKUP_EVERY_CHECKPOINTS checkpoints (30 minutes), starting with the first. + if (checkpoints++ % BACKUP_EVERY_CHECKPOINTS == 0) { + persistence.vacuumIntoBackup(); + } } }.runTaskTimer(VehicleFramework.plugin, 0L, 6000L); } diff --git a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManager.java b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManager.java index 7912102..269fbbb 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManager.java +++ b/src/main/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManager.java @@ -40,6 +40,7 @@ import net.tfminecraft.tlibs.TLibs; import net.tfminecraft.vehicleframework.VFLogger; import net.tfminecraft.vehicleframework.VehicleFramework; +import net.tfminecraft.vehicleframework.bones.ConvertedAngle; import net.tfminecraft.vehicleframework.cache.Cache; public final class TrackDisplayManager implements Listener { @@ -49,6 +50,9 @@ public final class TrackDisplayManager implements Listener { public static final String KEY_SWITCH = "junction_switch"; private final List live = new ArrayList<>(); + // The switch displays in live, with their junction, so the per-tick animation + // does not read the tags of every rail display. + private final List switches = new ArrayList<>(); private final Set missingLogged = new HashSet<>(); private final Deque resyncQueue = new ArrayDeque<>(); private CommandSender resyncNotify; @@ -301,6 +305,7 @@ public void despawnAll() { } } live.clear(); + switches.clear(); } public void spawnChunk(Chunk chunk) { @@ -567,6 +572,7 @@ public void despawnSwitch(UUID junctionId) { it.remove(); } } + switches.removeIf(entry -> junctionId.equals(entry.junctionId())); } private void spawnSwitchInChunk(TrackJunction junction, TrackSpline stem, Chunk chunk) { @@ -604,6 +610,7 @@ private void spawnSwitchInChunk(TrackJunction junction, TrackSpline stem, Chunk switchKey(), PersistentDataType.STRING, junction.id.toString()); }); live.add(display); + switches.add(new SwitchDisplay(display, junction.id)); } private static TrackSwitchPose switchPose(TrackSpline stem, TrackJunction junction) { @@ -626,21 +633,20 @@ private void tickSwitches() { return; } float maxDelta = Cache.trackSwitchThrowDegreesPerSecond / 20f; - Iterator it = live.iterator(); + Iterator it = switches.iterator(); while (it.hasNext()) { - ItemDisplay display = it.next(); - if (display == null || display.isDead() || !display.isValid()) { + SwitchDisplay entry = it.next(); + ItemDisplay display = entry.display(); + if (display.isDead() || !display.isValid()) { it.remove(); + live.remove(display); continue; } - UUID junctionId = readSwitchId(display); - if (junctionId == null) { - continue; - } - TrackJunction junction = registry.getJunction(junctionId).orElse(null); + TrackJunction junction = registry.getJunction(entry.junctionId()).orElse(null); if (junction == null || junction.branchSplineId == null) { display.remove(); it.remove(); + live.remove(display); continue; } TrackSpline stem = registry.get(junction.stemSplineId).orElse(null); @@ -648,8 +654,12 @@ private void tickSwitches() { if (pose == null) { continue; } - float next = TrackSwitchPose.stepYaw(display.getLocation().getYaw(), pose.targetYaw, maxDelta); Location loc = display.getLocation(); + float next = TrackSwitchPose.stepYaw(loc.getYaw(), pose.targetYaw, maxDelta); + // A switch already at its target would only resend the same position. + if (Math.abs(ConvertedAngle.shortestDelta(loc.getYaw(), next)) < 1e-4f) { + continue; + } loc.setYaw(next); display.teleport(loc); } @@ -789,6 +799,7 @@ private void despawnAllSwitches() { it.remove(); } } + switches.clear(); } private void tickResync() { @@ -847,6 +858,7 @@ private void removeTaggedInChunk(Chunk chunk) { } } live.removeIf(display -> display == null || display.isDead() || !display.isValid()); + switches.removeIf(entry -> entry.display().isDead() || !entry.display().isValid()); } private boolean isTrackDisplay(ItemDisplay display) { @@ -912,4 +924,7 @@ private Integer readEdge(ItemDisplay display) { private record ChunkRef(String world, int x, int z) { } + + private record SwitchDisplay(ItemDisplay display, UUID junctionId) { + } } diff --git a/src/test/java/net/tfminecraft/vehicleframework/database/DatabaseRuntimeCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/database/DatabaseRuntimeCoverageTest.java index 85da111..82f6daf 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/database/DatabaseRuntimeCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/database/DatabaseRuntimeCoverageTest.java @@ -53,6 +53,56 @@ class DatabaseRuntimeCoverageTest { } } + @Test void checkpointBatchCommitsEveryVehicleInOneTransactionAndKeepsPerVehicleFailures() { + VehicleRepository repository = VehicleRepository.open(directory.resolve("batch.db").toFile()); + try { + VehiclePersistence persistence = new VehiclePersistence(repository); + assertTrue(persistence.saveLiveBatch(null).isEmpty()); + assertTrue(persistence.saveLiveBatch(List.of()).isEmpty()); + ActiveVehicle first = VehiclePayloadCoverageTest.liveVehicle(); + ActiveVehicle second = VehiclePayloadCoverageTest.liveVehicle(); + String secondId = UUID.randomUUID().toString(); + when(second.getUUID()).thenReturn(secondId); + ActiveVehicle broken = VehiclePayloadCoverageTest.liveVehicle(); + when(broken.getUUID()).thenReturn(UUID.randomUUID().toString()); + when(broken.getEntity()).thenReturn(null); + Map results = persistence.saveLiveBatch(List.of(first, broken, second)); + assertEquals(List.of(first, broken, second), List.copyOf(results.keySet())); + assertTrue(results.get(first).isSaved()); + assertEquals("entity invalid, no SQLite row", results.get(broken).reason()); + assertTrue(results.get(second).isSaved()); + assertTrue(repository.findLive(ID).isPresent()); + assertTrue(repository.findLive(secondId).isPresent()); + assertTrue(repository.hasLiveInChunk("world", -2, -1)); + } finally { repository.close(); } + } + + @Test void checkpointBatchFallsBackToSingleSavesWhenTheTransactionFails() { + VehicleRepository repository = mock(VehicleRepository.class); + VehiclePersistence persistence = new VehiclePersistence(repository); + ActiveVehicle live = VehiclePayloadCoverageTest.liveVehicle(); + when(repository.saveLive(any())).thenReturn(true); + doAnswer(call -> { call.getArgument(0).run(); return null; }).when(repository).runInTransaction(any()); + assertTrue(persistence.saveLiveBatch(List.of(live)).get(live).isSaved()); + verify(repository, times(1)).runInTransaction(any()); + verify(repository, times(1)).saveLive(any()); + try (var logger = mockStatic(VFLogger.class)) { + doAnswer(call -> { call.getArgument(0).run(); throw new IllegalStateException("cannot commit"); }) + .when(repository).runInTransaction(any()); + assertTrue(persistence.saveLiveBatch(List.of(live)).get(live).isSaved()); + verify(repository, times(3)).saveLive(any()); + logger.verify(() -> VFLogger.log("SQLite checkpoint transaction failed, saving vehicles one by one: cannot commit")); + doThrow(new IllegalStateException()).when(repository).runInTransaction(any()); + assertTrue(persistence.saveLiveBatch(List.of(live)).get(live).isSaved()); + logger.verify(() -> VFLogger.log("SQLite checkpoint transaction failed, saving vehicles one by one")); + doThrow(new IllegalStateException(" ")).when(repository).runInTransaction(any()); + assertTrue(persistence.saveLiveBatch(List.of(live)).get(live).isSaved()); + logger.verify(() -> VFLogger.log("SQLite checkpoint transaction failed, saving vehicles one by one"), times(2)); + } + VehiclePersistResult missing = new VehiclePersistence(null).saveLiveBatch(List.of(live)).get(live); + assertEquals("SQLite is not open", missing.reason()); + } + @Test void missingRepositoryAndClosedDatabaseFailWithoutMutatingOrThrowing() { VehiclePersistence missing = new VehiclePersistence(null); assertTrue(missing.saveLiveResult(null).isFailed()); diff --git a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java index e095ad4..0e67eb3 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java @@ -58,6 +58,7 @@ static void initializeSounds() { private final List scheduled = new ArrayList<>(); private final List timers = new ArrayList<>(); private MockedStatic bukkit; + private MockedStatic logger; private MockedStatic storage; private MockedStatic loader; private MockedStatic spawning; @@ -93,7 +94,7 @@ void setup() { bukkit.when(Bukkit::getPluginManager).thenReturn(plugins); bukkit.when(Bukkit::getScheduler).thenReturn(scheduler); bukkit.when(Bukkit::getOnlinePlayers).thenReturn(List.of()); - keep(mockStatic(VFLogger.class)); + logger = keep(mockStatic(VFLogger.class)); keep(mockStatic(PersistenceLog.class)); keep(mockStatic(ConsistRelinker.class)); keep(mockStatic(TrainCollision.class)); @@ -466,9 +467,25 @@ void timersRunIndependentTicksAndSkipUnavailablePersistence() throws Exception { manager.towSelect(player, v); when(v.getEntity().getLocation()).thenReturn(new Location(world, 100, 64, 0)); assertDoesNotThrow(() -> timers.get(0).run()); assertDoesNotThrow(() -> timers.get(1).run()); assertTrue(map("tow").isEmpty()); - VehiclePersistence persistence = persistence(); when(persistence.saveLiveResult(v)).thenReturn(VehiclePersistResult.failed("full")); + VehiclePersistence persistence = persistence(); ActiveVehicle saved = vehicle("saved"); register(saved); + when(persistence.saveLiveBatch(anyCollection())).thenAnswer(call -> { + Map results = new LinkedHashMap<>(); + for (ActiveVehicle each : call.>getArgument(0)) + results.put(each, each == v ? VehiclePersistResult.failed("full") : VehiclePersistResult.saved()); + return results; + }); timers.get(2).run(); verify(persistence).checkpointWal(false); verify(persistence).vacuumIntoBackup(); - when(v.isDestroyed()).thenReturn(true); timers.get(2).run(); verify(persistence, times(1)).saveLiveResult(v); + verify(persistence, never()).saveLiveResult(any()); + String failed = "Failed to persist " + VehicleManager.describeVehicle(v) + " at " + VehicleManager.describeLocation(v) + " during checkpoint: full"; + logger.verify(() -> VFLogger.log(failed)); + logger.verify(() -> VFLogger.log(contains("'Cart saved'")), never()); + when(v.isDestroyed()).thenReturn(true); timers.get(2).run(); + verify(persistence).saveLiveBatch(List.of(saved)); + // The backup copy runs on the first checkpoint and then every BACKUP_EVERY_CHECKPOINTS. + for (int i = 2; i < VehicleManager.BACKUP_EVERY_CHECKPOINTS; i++) timers.get(2).run(); + verify(persistence, times(1)).vacuumIntoBackup(); + timers.get(2).run(); verify(persistence, times(2)).vacuumIntoBackup(); + verify(persistence, times(VehicleManager.BACKUP_EVERY_CHECKPOINTS + 1)).checkpointWal(false); manager.reload(); verify(manager.getSpawnManager()).reload(); } diff --git a/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManagerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManagerCoverageTest.java index 08ba457..604c223 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/tracks/TrackDisplayManagerCoverageTest.java @@ -505,6 +505,35 @@ void switchesUseConfiguredItemPositionAndAnimateTowardChangedThrow() { assertTrue(spawned.getLast().removed); } + @Test + void switchTickSkipsRailTagsAndIdleSwitches() { + TrackSpline rail = rail(); + TrackJunction junction = junction(rail); + DisplayState track = spawn(rail.getId(), visual(0, 8)); + manager.spawnSwitchesForSpline(rail); + DisplayState lever = spawned.getLast(); + PersistentDataContainer railTags = track.entity.getPersistentDataContainer(); + PersistentDataContainer leverTags = lever.entity.getPersistentDataContainer(); + clearInvocations(railTags, leverTags); + tick.run(); + verifyNoInteractions(railTags, leverTags); + verify(lever.entity, never()).teleport(any(Location.class)); + junctions.put(junction.id, junction.withThrown(true)); + tick.run(); + verify(lever.entity, times(1)).teleport(any(Location.class)); + for (int i = 0; i < 200; i++) tick.run(); + float settled = lever.location.getYaw(); + int moves = mockingDetails(lever.entity).getInvocations().stream() + .filter(call -> call.getMethod().getName().equals("teleport")).toList().size(); + tick.run(); + assertEquals(settled, lever.location.getYaw()); + verify(lever.entity, times(moves)).teleport(any(Location.class)); + manager.onChunkUnload(new ChunkUnloadEvent(chunk(0, 0))); + assertTrue(lever.removed); + tick.run(); + verify(lever.entity, times(moves)).teleport(any(Location.class)); + } + @Test void switchReloadPreservesRailsReplacesSwitchesAndClearsItemCache() { TrackSpline rail = rail();