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();