From 4345e9a324768957f982412eaa7eb8a82e8ce21c Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:39:08 +0100 Subject: [PATCH 1/2] perf: flush the sales folder's parent only when the folder is new Every market sale flushed three folders to disk on the main thread: the plugin folder (in case pending-sales had just been created), then pending-sales after writing the sale's intent file and again after removing it. Spark showed 4.5 s in recording sales and 3.3 s in folder flushes over 9 hours, up to 0.5 s in one minute. The plugin folder only needs flushing when pending-sales was just made, so a crash cannot lose the new folder and the intent inside it. A sale now checks whether the folder exists first and flushes the parent only when it creates it, plus once per server run for a folder that already existed (covering an earlier run whose flush failed). Every later sale skips that flush. The intent file is still written with CREATE_NEW and flushed, and both pending-sales flushes stay, in the same order as before. The flush after removing the intent stays on purpose: there is no automatic reconcile, so an intent that came back after a crash would make the seller's next sale fail its CREATE_NEW and tell them it needs staff review, until staff removed the file. Co-Authored-By: Claude Opus 5.5 --- .../marketblock/trade/MarketSale.java | 28 +++++++++++++--- .../marketblock/trade/MarketSaleTest.java | 32 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java b/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java index cdb2b45..b6a03e6 100644 --- a/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java +++ b/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java @@ -7,7 +7,9 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.Properties; +import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import net.tfminecraft.denareconomy.api.Denars; import net.tfminecraft.denareconomy.api.Result; @@ -20,6 +22,9 @@ * after the payment is applied. */ public final class MarketSale { + /** Sale folders whose entry in their parent folder this server run has flushed to disk. */ + private static final Set FLUSHED_FOLDERS = ConcurrentHashMap.newKeySet(); + private final String opId; private final CharacterRef seller; private final SaleTake take; @@ -53,15 +58,24 @@ public Result settle() throws IOException { } if (result == Result.APPLIED || result == Result.REFUSED) { Files.deleteIfExists(intent); + // Must stay: an intent that comes back after a crash blocks the seller's next sale (CREATE_NEW). syncDirectory(); } return result; } private void record() throws IOException { - Files.createDirectories(intent.getParent()); - try (FileChannel parent = FileChannel.open(intent.getParent().getParent(), StandardOpenOption.READ)) { - parent.force(true); + Path folder = intent.getParent(); + Path key = folder.toAbsolutePath().normalize(); + boolean created = !Files.isDirectory(folder); + if (created) { + Files.createDirectories(folder); + } + // A new folder survives a crash only once its parent is flushed. An existing one is flushed once per + // run, in case the flush after it was made failed; after that its entry is already on disk. + if (created || !FLUSHED_FOLDERS.contains(key)) { + force(folder.getParent()); + FLUSHED_FOLDERS.add(key); } Properties data = new Properties(); data.setProperty("operation", opId); @@ -80,8 +94,12 @@ private void record() throws IOException { } private void syncDirectory() throws IOException { - try (FileChannel directory = FileChannel.open(intent.getParent(), StandardOpenOption.READ)) { - directory.force(true); + force(intent.getParent()); + } + + static void force(Path directory) throws IOException { + try (FileChannel channel = FileChannel.open(directory, StandardOpenOption.READ)) { + channel.force(true); } } } diff --git a/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java b/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java index 492191f..d4c8393 100644 --- a/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java +++ b/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java @@ -117,6 +117,38 @@ void paidRetryUsesStoredResultAfterTaxChangesAndRefusesChangedGoods() throws Exc } + @Test + void theParentFolderIsFlushedOnlyForANewSaleFolderOrOnceARunForAnExistingOne() throws Exception { + var seller = money.play(player); + player.getInventory().setItem(0, new ItemStack(Material.CARROT, 12)); + Path sales = folder.resolve("sales"); + Path existing = java.nio.file.Files.createDirectories(folder.resolve("existing")); + try (var flushes = mockStatic(MarketSale.class, CALLS_REAL_METHODS)) { + flushes.when(() -> MarketSale.force(any(Path.class))).thenAnswer(call -> null); + + assertEquals(Result.APPLIED, sell(sales, seller)); + assertEquals(Result.APPLIED, sell(sales, seller)); + flushes.verify(() -> MarketSale.force(folder), times(1)); + // Each sale still flushes its intent being written and being removed. + flushes.verify(() -> MarketSale.force(sales), times(4)); + + assertEquals(Result.APPLIED, sell(existing, seller)); + assertEquals(Result.APPLIED, sell(existing, seller)); + flushes.verify(() -> MarketSale.force(folder), times(2)); + + java.nio.file.Files.delete(sales); + assertEquals(Result.APPLIED, sell(sales, seller)); + flushes.verify(() -> MarketSale.force(folder), times(3)); + assertTrue(java.nio.file.Files.isDirectory(sales)); + } + assertEquals(2, player.getInventory().getItem(0).getAmount()); + assertEquals(500, Denars.balance(Denars.pouch(seller))); + } + + private Result sell(Path sales, net.tfminecraft.tlibs.character.CharacterRef seller) throws Exception { + return new MarketSale(sales, seller, InventoryUtils.planTake(player, "v.carrot", 2), 100).settle(); + } + @Test void waitingForFactionTaxDoesNotCreateAnAmbiguousSale() throws Exception { var seller=money.play(player); MockBukkit.createMockPlugin("SimpleFactions"); From 79f7d7cd704fd6214aa034e3567a75b5c9c334f0 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:12:22 +0100 Subject: [PATCH 2/2] fix: forget an earlier flush when the sales folder is recreated If pending-sales was deleted and recreated during a run, a failed parent flush left the old folder marked as flushed, so the next sale could pay without a durable folder entry. Recreating the folder now clears the mark until its own flush succeeds (CodeRabbit). Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/marketblock/trade/MarketSale.java | 2 ++ .../marketblock/trade/MarketSaleTest.java | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java b/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java index b6a03e6..cf4dd28 100644 --- a/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java +++ b/src/main/java/net/tfminecraft/marketblock/trade/MarketSale.java @@ -69,6 +69,8 @@ private void record() throws IOException { Path key = folder.toAbsolutePath().normalize(); boolean created = !Files.isDirectory(folder); if (created) { + // A recreated folder is a new entry: forget the earlier flush until this one succeeds. + FLUSHED_FOLDERS.remove(key); Files.createDirectories(folder); } // A new folder survives a crash only once its parent is flushed. An existing one is flushed once per diff --git a/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java b/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java index d4c8393..0079e4d 100644 --- a/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java +++ b/src/test/java/net/tfminecraft/marketblock/trade/MarketSaleTest.java @@ -120,7 +120,7 @@ void paidRetryUsesStoredResultAfterTaxChangesAndRefusesChangedGoods() throws Exc @Test void theParentFolderIsFlushedOnlyForANewSaleFolderOrOnceARunForAnExistingOne() throws Exception { var seller = money.play(player); - player.getInventory().setItem(0, new ItemStack(Material.CARROT, 12)); + player.getInventory().setItem(0, new ItemStack(Material.CARROT, 14)); Path sales = folder.resolve("sales"); Path existing = java.nio.file.Files.createDirectories(folder.resolve("existing")); try (var flushes = mockStatic(MarketSale.class, CALLS_REAL_METHODS)) { @@ -140,9 +140,17 @@ void theParentFolderIsFlushedOnlyForANewSaleFolderOrOnceARunForAnExistingOne() t assertEquals(Result.APPLIED, sell(sales, seller)); flushes.verify(() -> MarketSale.force(folder), times(3)); assertTrue(java.nio.file.Files.isDirectory(sales)); + + // A recreated folder whose flush fails is flushed again by the next sale, before it pays. + java.nio.file.Files.delete(sales); + flushes.when(() -> MarketSale.force(folder)).thenThrow(new java.io.IOException("simulated parent flush failure")); + assertThrows(java.io.IOException.class, () -> sell(sales, seller)); + flushes.when(() -> MarketSale.force(folder)).thenAnswer(call -> null); + assertEquals(Result.APPLIED, sell(sales, seller)); + flushes.verify(() -> MarketSale.force(folder), times(5)); } assertEquals(2, player.getInventory().getItem(0).getAmount()); - assertEquals(500, Denars.balance(Denars.pouch(seller))); + assertEquals(600, Denars.balance(Denars.pouch(seller))); } private Result sell(Path sales, net.tfminecraft.tlibs.character.CharacterRef seller) throws Exception {