From 6c7796f12f2d494e2e2d485d74b202617ce24b18 Mon Sep 17 00:00:00 2001 From: Brian Neumann-Fopiano Date: Tue, 29 Sep 2026 00:29:34 -0400 Subject: [PATCH] Pregen failure handling - Chunk generator reports generation failures to the running pregen job - Pregen methods forward generation failures to the async method - Async pregen fails every outstanding request through an in-flight ledger - In-flight requests settle once and ignore late completions - Pregen aborts with the failing chunk as its cause - Admission drain waits with a 60 second deadline - Closing the admission gate fails leftover requests - Tests cover failure propagation, the ledger and the drain deadline --- .../generation/BukkitChunkGenerator.java | 10 ++ .../iris/studio/view/PregeneratorJob.java | 8 + .../pregen/AsyncOrMedievalPregenMethod.java | 5 + .../iris/world/pregen/AsyncPregenMethod.java | 88 ++++++++++- .../iris/world/pregen/CachedPregenMethod.java | 5 + .../iris/world/pregen/HybridPregenMethod.java | 5 + .../iris/world/pregen/IrisPregenerator.java | 4 + .../world/pregen/PregenAdmissionGate.java | 14 +- .../world/pregen/PregenInFlightRequests.java | 47 ++++++ .../iris/world/pregen/PregeneratorMethod.java | 3 + .../AsyncPregenGenerationFailureTest.java | 147 ++++++++++++++++++ .../AsyncPregenMethodConcurrencyCapTest.java | 8 +- .../AsyncPregenServerSaveOwnershipTest.java | 2 + .../pregen/AsyncPregenWorkerBoostTest.java | 3 +- .../world/pregen/PregenAdmissionGateTest.java | 39 ++++- .../pregen/PregenInFlightRequestsTest.java | 62 ++++++++ 16 files changed, 436 insertions(+), 14 deletions(-) create mode 100644 core/src/main/java/art/arcane/iris/world/pregen/PregenInFlightRequests.java create mode 100644 core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenGenerationFailureTest.java create mode 100644 core/src/test/java/art/arcane/iris/world/pregen/PregenInFlightRequestsTest.java diff --git a/core/src/main/java/art/arcane/iris/platform/generation/BukkitChunkGenerator.java b/core/src/main/java/art/arcane/iris/platform/generation/BukkitChunkGenerator.java index 556dcce12..7b03b79e6 100644 --- a/core/src/main/java/art/arcane/iris/platform/generation/BukkitChunkGenerator.java +++ b/core/src/main/java/art/arcane/iris/platform/generation/BukkitChunkGenerator.java @@ -30,6 +30,7 @@ import java.util.concurrent.Semaphore; import art.arcane.iris.spi.IrisServices; import art.arcane.iris.platform.bukkit.BukkitPlatform; import art.arcane.iris.configuration.IrisSettings; +import art.arcane.iris.studio.view.PregeneratorJob; import art.arcane.iris.world.IrisWorlds; import art.arcane.iris.pack.loading.IrisData; import art.arcane.iris.platform.bukkit.nms.INMS; @@ -1290,6 +1291,15 @@ public class BukkitChunkGenerator extends ChunkGenerator implements PlatformChun @Override public void generateNoise(@NotNull WorldInfo world, @NotNull Random random, int x, int z, @NotNull ChunkGenerator.ChunkData d) { + try { + generateNoiseChunk(world, random, x, z, d); + } catch (Throwable e) { + PregeneratorJob.reportChunkGenerationFailure(this.world.identity(), x, z, e); + throw e; + } + } + + private void generateNoiseChunk(WorldInfo world, Random random, int x, int z, ChunkGenerator.ChunkData d) { if (closing) { throw new IllegalStateException("Iris chunk generation was rejected while the generator is closing."); } diff --git a/core/src/main/java/art/arcane/iris/studio/view/PregeneratorJob.java b/core/src/main/java/art/arcane/iris/studio/view/PregeneratorJob.java index f767a73e6..afe412d8f 100644 --- a/core/src/main/java/art/arcane/iris/studio/view/PregeneratorJob.java +++ b/core/src/main/java/art/arcane/iris/studio/view/PregeneratorJob.java @@ -254,6 +254,14 @@ public class PregeneratorJob implements PregenListener, PregenRenderSource { return instance.get(); } + public static void reportChunkGenerationFailure(String worldIdentity, int chunkX, int chunkZ, Throwable failure) { + PregeneratorJob inst = instance.get(); + if (inst == null || worldIdentity == null || !worldIdentity.equals(inst.worldIdentity())) { + return; + } + inst.pregenerator.onChunkGenerationFailed(chunkX, chunkZ, failure); + } + public static boolean pauseResume() { PregeneratorJob inst = instance.get(); return inst != null && inst.togglePause(); diff --git a/core/src/main/java/art/arcane/iris/world/pregen/AsyncOrMedievalPregenMethod.java b/core/src/main/java/art/arcane/iris/world/pregen/AsyncOrMedievalPregenMethod.java index 0a729c1fc..e4f50bd83 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/AsyncOrMedievalPregenMethod.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/AsyncOrMedievalPregenMethod.java @@ -85,6 +85,11 @@ public class AsyncOrMedievalPregenMethod implements PregeneratorMethod { method.onRegionBounds(minRegionX, minRegionZ, maxRegionX, maxRegionZ); } + @Override + public void onChunkGenerationFailed(int chunkX, int chunkZ, Throwable failure) { + method.onChunkGenerationFailed(chunkX, chunkZ, failure); + } + @Override public void onPregenStart(int centerBlockX, int centerBlockZ) { method.onPregenStart(centerBlockX, centerBlockZ); diff --git a/core/src/main/java/art/arcane/iris/world/pregen/AsyncPregenMethod.java b/core/src/main/java/art/arcane/iris/world/pregen/AsyncPregenMethod.java index b470b333d..9d780aaee 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/AsyncPregenMethod.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/AsyncPregenMethod.java @@ -21,6 +21,7 @@ package art.arcane.iris.world.pregen; import art.arcane.iris.spi.IrisLogging; import art.arcane.iris.world.lifecycle.WorldLifecycleService; import art.arcane.volmlib.nativelib.terrain.NativeWorkerPool; +import java.util.Map; import java.util.Objects; import art.arcane.iris.world.IrisPaperLikeBackendMode; import art.arcane.iris.world.IrisRuntimeSchedulerMode; @@ -55,6 +56,7 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; public class AsyncPregenMethod implements PregeneratorMethod { // THREAD_COUNT records the pool's original size while boosted; BOOST_HOLDERS counts live @@ -64,7 +66,8 @@ public class AsyncPregenMethod implements PregeneratorMethod { private static final AtomicInteger BOOST_HOLDERS = new AtomicInteger(); private static final int ADAPTIVE_SLOW_REQUEST_STEP = 3; private static final int ADAPTIVE_RECOVERY_INTERVAL = 8; - private static final long CLOSE_DRAIN_WARNING_SECONDS = 60L; + private static final long CLOSE_DRAIN_WARNING_SECONDS = 20L; + private static final long CLOSE_DRAIN_TIMEOUT_SECONDS = 60L; private static final long FLUSH_TIMEOUT_SECONDS = 120L; private static final long MANTLE_CLEANUP_DRAIN_SECONDS = 60L; private static final long CHUNK_FLUSH_DRAIN_SECONDS = 30L; @@ -113,6 +116,8 @@ public class AsyncPregenMethod implements PregeneratorMethod { private final AtomicLong failed = new AtomicLong(); private final AtomicLong lastProgressAt = new AtomicLong(M.ms()); private final AtomicBoolean closing = new AtomicBoolean(); + private final PregenInFlightRequests inFlightRequests = new PregenInFlightRequests(); + private final AtomicReference generationFailure = new AtomicReference<>(); private final AtomicBoolean holdsWorkerBoost = new AtomicBoolean(); private volatile Engine metricsEngine; private volatile Mantle cachedMantle; @@ -449,10 +454,20 @@ public class AsyncPregenMethod implements PregeneratorMethod { } private void completeChunk(int x, int z, PregenListener listener, Chunk chunk, Throwable throwable) { + PregenListener owner = inFlightRequests.settle(PregenInFlightRequests.key(x, z)); + if (owner == null) { + return; + } + settleChunk(x, z, owner, chunk, throwable, true); + } + + private void settleChunk(int x, int z, PregenListener listener, Chunk chunk, Throwable throwable, boolean reportFailure) { boolean success = false; try { if (throwable != null) { - onChunkFutureFailure(x, z, throwable); + if (reportFailure) { + onChunkFutureFailure(x, z, throwable); + } onChunkFailedToLoad(x, z); listener.onChunkFailed(x, z); } else if (chunk == null) { @@ -652,11 +667,57 @@ public class AsyncPregenMethod implements PregeneratorMethod { + " stalledForMs=" + stalledFor; } - private void markSubmitted() { + private void markSubmitted(int x, int z, PregenListener listener) { + inFlightRequests.add(PregenInFlightRequests.key(x, z), listener); submitted.incrementAndGet(); inFlight.incrementAndGet(); } + @Override + public void onChunkGenerationFailed(int chunkX, int chunkZ, Throwable failure) { + GenerationFailure recorded = new GenerationFailure(chunkX, chunkZ, failure); + if (generationFailure.compareAndSet(null, recorded)) { + IrisLogging.error("Async pregen aborting: Iris failed to generate chunk " + chunkX + "," + chunkZ + + " and the server's chunk system will not complete the " + inFlightRequests.size() + + " outstanding chunk request(s). " + metricsSnapshot()); + } + failOutstandingRequests(failure); + } + + private void failOutstandingRequests(Throwable failure) { + Map outstanding = inFlightRequests.drain(); + for (Map.Entry entry : outstanding.entrySet()) { + long key = entry.getKey(); + settleChunk(PregenInFlightRequests.chunkX(key), PregenInFlightRequests.chunkZ(key), entry.getValue(), + null, failure, false); + } + admission.wake(); + backpressure.signalProgress(); + } + + private void throwIfGenerationFailed() { + GenerationFailure failure = generationFailure.get(); + if (failure != null) { + throw new IllegalStateException("Pregen aborted: Iris failed to generate chunk " + failure.chunkX() + "," + + failure.chunkZ() + " in world " + world.getName() + + " and the server's chunk system stopped completing requests.", failure.cause()); + } + } + + private void abandonOutstandingRequests(PregenAdmissionGate.Drain drain) { + Map abandoned = inFlightRequests.drain(); + IrisLogging.error("Async pregen abandoned " + abandoned.size() + " chunk request(s) that did not complete within " + + CLOSE_DRAIN_TIMEOUT_SECONDS + "s of closing (" + drain.outstanding() + + " permit(s) still held). They are not cached and regenerate on the next run. " + metricsSnapshot()); + IllegalStateException cause = new IllegalStateException( + "Async chunk request never completed before the pregen closed in world " + world.getName() + "."); + for (Map.Entry entry : abandoned.entrySet()) { + long key = entry.getKey(); + settleChunk(PregenInFlightRequests.chunkX(key), PregenInFlightRequests.chunkZ(key), entry.getValue(), + null, cause, false); + } + } + private void markFinished(boolean success) { if (success) { completed.incrementAndGet(); @@ -774,11 +835,17 @@ public class AsyncPregenMethod implements PregeneratorMethod { // A stop request interrupts the pregen worker; shield the drain and flush so chunks still hit disk. boolean interrupted = Thread.interrupted(); try { - interrupted |= admission.awaitDrain( + PregenAdmissionGate.Drain drain = admission.awaitDrain( CLOSE_DRAIN_WARNING_SECONDS, TimeUnit.SECONDS, + CLOSE_DRAIN_TIMEOUT_SECONDS, + TimeUnit.SECONDS, () -> IrisLogging.warn("Async pregen is still draining outstanding chunks. " + metricsSnapshot()) ); + interrupted |= drain.interrupted(); + if (!drain.drained()) { + abandonOutstandingRequests(drain); + } mantleCleanup.close(MANTLE_CLEANUP_DRAIN_SECONDS, TimeUnit.SECONDS); flushAllRemainingChunks(); @@ -808,7 +875,7 @@ public class AsyncPregenMethod implements PregeneratorMethod { } private boolean isCancelled() { - return closing.get() || Thread.currentThread().isInterrupted(); + return closing.get() || generationFailure.get() != null || Thread.currentThread().isInterrupted(); } @Override @@ -835,6 +902,7 @@ public class AsyncPregenMethod implements PregeneratorMethod { @Override public void generateChunk(int x, int z, PregenListener listener) { + throwIfGenerationFailed(); listener.onChunkGenerating(x, z); if (MantleHeapPressure.overHighWater()) { reclaimMemory(); @@ -865,9 +933,14 @@ public class AsyncPregenMethod implements PregeneratorMethod { recordPermitWait(waited.permitMs()); } + throwIfGenerationFailed(); regionPending.computeIfAbsent(rkey(x >> 5, z >> 5), k -> new AtomicInteger(1)).incrementAndGet(); - markSubmitted(); + markSubmitted(x, z, listener); executor.generate(x, z, listener); + GenerationFailure failure = generationFailure.get(); + if (failure != null) { + failOutstandingRequests(failure.cause()); + } } private void reclaimHeapPressure() { @@ -1104,6 +1177,9 @@ public class AsyncPregenMethod implements PregeneratorMethod { } } + private record GenerationFailure(int chunkX, int chunkZ, Throwable cause) { + } + private record ChunkAsyncMethodSelection(Method urgentMethod, Method standardMethod, String mode) { } } diff --git a/core/src/main/java/art/arcane/iris/world/pregen/CachedPregenMethod.java b/core/src/main/java/art/arcane/iris/world/pregen/CachedPregenMethod.java index 783906055..ecd559cd7 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/CachedPregenMethod.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/CachedPregenMethod.java @@ -189,6 +189,11 @@ public class CachedPregenMethod implements PregeneratorMethod { method.onRegionBounds(minRegionX, minRegionZ, maxRegionX, maxRegionZ); } + @Override + public void onChunkGenerationFailed(int chunkX, int chunkZ, Throwable failure) { + method.onChunkGenerationFailed(chunkX, chunkZ, failure); + } + @Override public void onPregenStart(int centerBlockX, int centerBlockZ) { method.onPregenStart(centerBlockX, centerBlockZ); diff --git a/core/src/main/java/art/arcane/iris/world/pregen/HybridPregenMethod.java b/core/src/main/java/art/arcane/iris/world/pregen/HybridPregenMethod.java index 7e7498bf8..e1855d7f9 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/HybridPregenMethod.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/HybridPregenMethod.java @@ -78,6 +78,11 @@ public class HybridPregenMethod implements PregeneratorMethod { inWorld.onRegionBounds(minRegionX, minRegionZ, maxRegionX, maxRegionZ); } + @Override + public void onChunkGenerationFailed(int chunkX, int chunkZ, Throwable failure) { + inWorld.onChunkGenerationFailed(chunkX, chunkZ, failure); + } + @Override public void onPregenStart(int centerBlockX, int centerBlockZ) { inWorld.onPregenStart(centerBlockX, centerBlockZ); diff --git a/core/src/main/java/art/arcane/iris/world/pregen/IrisPregenerator.java b/core/src/main/java/art/arcane/iris/world/pregen/IrisPregenerator.java index 61f45c3e4..1b56eaf85 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/IrisPregenerator.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/IrisPregenerator.java @@ -215,6 +215,10 @@ public class IrisPregenerator { signalState(); } + public void onChunkGenerationFailed(int chunkX, int chunkZ, Throwable failure) { + generator.onChunkGenerationFailed(chunkX, chunkZ, failure); + } + public void start() { PrecisionStopwatch p = PrecisionStopwatch.start(); boolean completed = false; diff --git a/core/src/main/java/art/arcane/iris/world/pregen/PregenAdmissionGate.java b/core/src/main/java/art/arcane/iris/world/pregen/PregenAdmissionGate.java index a4533d99d..0b4be13d2 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/PregenAdmissionGate.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/PregenAdmissionGate.java @@ -111,14 +111,20 @@ public final class PregenAdmissionGate { } } - public boolean awaitDrain(long warningInterval, TimeUnit timeUnit, Runnable onWait) { + public Drain awaitDrain(long warningInterval, TimeUnit warningUnit, long deadline, TimeUnit deadlineUnit, Runnable onWait) { + long warningNanos = warningUnit.toNanos(warningInterval); + long deadlineAt = System.nanoTime() + deadlineUnit.toNanos(deadline); boolean interrupted = false; lock.lock(); try { while (free < permits) { + long remaining = deadlineAt - System.nanoTime(); + if (remaining <= 0L) { + return new Drain(false, interrupted, permits - free); + } boolean signalled = false; try { - signalled = changed.await(warningInterval, timeUnit); + signalled = changed.awaitNanos(Math.min(warningNanos, remaining)) > 0L; } catch (InterruptedException e) { interrupted = true; continue; @@ -127,10 +133,12 @@ public final class PregenAdmissionGate { onWait.run(); } } + return new Drain(true, interrupted, 0); } finally { lock.unlock(); } + } - return interrupted; + public record Drain(boolean drained, boolean interrupted, int outstanding) { } } diff --git a/core/src/main/java/art/arcane/iris/world/pregen/PregenInFlightRequests.java b/core/src/main/java/art/arcane/iris/world/pregen/PregenInFlightRequests.java new file mode 100644 index 000000000..d184cd1a4 --- /dev/null +++ b/core/src/main/java/art/arcane/iris/world/pregen/PregenInFlightRequests.java @@ -0,0 +1,47 @@ +package art.arcane.iris.world.pregen; + +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; + +public final class PregenInFlightRequests { + private final ConcurrentHashMap requests = new ConcurrentHashMap<>(); + + public static long key(int chunkX, int chunkZ) { + return ((long) chunkX << 32) | (chunkZ & 0xFFFFFFFFL); + } + + public static int chunkX(long key) { + return (int) (key >> 32); + } + + public static int chunkZ(long key) { + return (int) key; + } + + public boolean add(long key, PregenListener listener) { + return requests.putIfAbsent(key, Objects.requireNonNull(listener, "listener")) == null; + } + + public PregenListener settle(long key) { + return requests.remove(key); + } + + public Map drain() { + HashMap drained = new HashMap<>(); + Iterator> iterator = requests.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry entry = iterator.next(); + if (requests.remove(entry.getKey(), entry.getValue())) { + drained.put(entry.getKey(), entry.getValue()); + } + } + return drained; + } + + public int size() { + return requests.size(); + } +} diff --git a/core/src/main/java/art/arcane/iris/world/pregen/PregeneratorMethod.java b/core/src/main/java/art/arcane/iris/world/pregen/PregeneratorMethod.java index 151ef6a15..ee0749620 100644 --- a/core/src/main/java/art/arcane/iris/world/pregen/PregeneratorMethod.java +++ b/core/src/main/java/art/arcane/iris/world/pregen/PregeneratorMethod.java @@ -83,6 +83,9 @@ public interface PregeneratorMethod { default void onRegionBounds(int minRegionX, int minRegionZ, int maxRegionX, int maxRegionZ) { } + default void onChunkGenerationFailed(int chunkX, int chunkZ, Throwable failure) { + } + default void onPregenStart(int centerBlockX, int centerBlockZ) { } diff --git a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenGenerationFailureTest.java b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenGenerationFailureTest.java new file mode 100644 index 000000000..3d3606a4f --- /dev/null +++ b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenGenerationFailureTest.java @@ -0,0 +1,147 @@ +package art.arcane.iris.world.pregen; + +import org.bukkit.Chunk; +import org.bukkit.World; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class AsyncPregenGenerationFailureTest { + @Test + public void aGenerationFailureFailsEveryOutstandingRequestAndReleasesTheirPermits() throws Exception { + Harness harness = Harness.withOutstanding(2); + IllegalStateException cause = new IllegalStateException("Accepted hydrology layer exceeds the mantle bounds"); + + harness.method.onChunkGenerationFailed(48, -4, cause); + + assertEquals(2, harness.admission.availablePermits()); + assertEquals(2, harness.failed.get()); + assertEquals(0, harness.inFlight.get()); + assertEquals(0, harness.requests.size()); + verify(harness.listener).onChunkFailed(10, 0); + verify(harness.listener).onChunkFailed(11, 0); + + IllegalStateException abort = assertThrows(IllegalStateException.class, + () -> harness.method.generateChunk(12, 0, harness.listener)); + assertSame(cause, abort.getCause()); + verify(harness.listener, never()).onChunkGenerating(12, 0); + } + + @Test + public void aLateCompletionForASettledRequestIsIgnored() throws Exception { + Harness harness = Harness.withOutstanding(2); + harness.method.onChunkGenerationFailed(48, -4, new IllegalStateException("boom")); + + harness.completeChunk(10, 0, null, null); + harness.completeChunk(11, 0, null, new IllegalStateException("late failure")); + + assertEquals(2, harness.failed.get()); + assertEquals(0, harness.completed.get()); + assertEquals(0, harness.inFlight.get()); + assertEquals(2, harness.admission.availablePermits()); + } + + @Test + public void closingAbandonsRequestsThatNeverCompleteAndReturnsTheirPermits() throws Exception { + Harness harness = Harness.withOutstanding(2); + + harness.abandon(new PregenAdmissionGate.Drain(false, false, 2)); + + assertEquals(2, harness.admission.availablePermits()); + assertEquals(2, harness.failed.get()); + assertEquals(0, harness.requests.size()); + verify(harness.listener).onChunkFailed(10, 0); + verify(harness.listener).onChunkFailed(11, 0); + } + + private static final class Harness { + private final AsyncPregenMethod method; + private final PregenAdmissionGate admission; + private final PregenInFlightRequests requests; + private final PregenListener listener; + private final AtomicInteger inFlight; + private final AtomicLong failed; + private final AtomicLong completed; + + private Harness(AsyncPregenMethod method, PregenAdmissionGate admission, PregenInFlightRequests requests, + PregenListener listener, AtomicInteger inFlight, AtomicLong failed, AtomicLong completed) { + this.method = method; + this.admission = admission; + this.requests = requests; + this.listener = listener; + this.inFlight = inFlight; + this.failed = failed; + this.completed = completed; + } + + static Harness withOutstanding(int outstanding) throws Exception { + AsyncPregenMethod method = mock(AsyncPregenMethod.class, CALLS_REAL_METHODS); + World world = mock(World.class); + when(world.getName()).thenReturn("qa"); + PregenAdmissionGate admission = new PregenAdmissionGate(outstanding, 50L, System::currentTimeMillis); + PregenInFlightRequests requests = new PregenInFlightRequests(); + PregenListener listener = mock(PregenListener.class); + AtomicInteger inFlight = new AtomicInteger(outstanding); + AtomicLong failed = new AtomicLong(); + AtomicLong completed = new AtomicLong(); + for (int index = 0; index < outstanding; index++) { + assertNotNull(admission.admit(() -> false, () -> false)); + requests.add(PregenInFlightRequests.key(10 + index, 0), listener); + } + set(method, "world", world); + set(method, "admission", admission); + set(method, "backpressure", mock(PregenMantleBackpressure.class)); + set(method, "mantleCleanup", mock(PregenSerialWorker.class)); + set(method, "inFlightRequests", requests); + set(method, "generationFailure", new AtomicReference<>()); + set(method, "regionPending", new ConcurrentHashMap<>()); + set(method, "lastFailedReleaseLogAt", new AtomicLong()); + set(method, "slowRequestWarnIntervalMs", 15_000); + set(method, "inFlight", inFlight); + set(method, "submitted", new AtomicLong(outstanding)); + set(method, "completed", completed); + set(method, "failed", failed); + set(method, "lastProgressAt", new AtomicLong()); + set(method, "adaptiveInFlightLimit", new AtomicInteger(outstanding)); + set(method, "threads", outstanding); + set(method, "closing", new AtomicBoolean()); + return new Harness(method, admission, requests, listener, inFlight, failed, completed); + } + + void completeChunk(int x, int z, Chunk chunk, Throwable throwable) throws Exception { + Method complete = AsyncPregenMethod.class.getDeclaredMethod( + "completeChunk", int.class, int.class, PregenListener.class, Chunk.class, Throwable.class); + complete.setAccessible(true); + complete.invoke(method, x, z, listener, chunk, throwable); + } + + void abandon(PregenAdmissionGate.Drain drain) throws Exception { + Method abandon = AsyncPregenMethod.class.getDeclaredMethod( + "abandonOutstandingRequests", PregenAdmissionGate.Drain.class); + abandon.setAccessible(true); + abandon.invoke(method, drain); + } + + private static void set(AsyncPregenMethod method, String name, Object value) throws Exception { + Field field = AsyncPregenMethod.class.getDeclaredField(name); + field.setAccessible(true); + field.set(method, value); + } + } +} diff --git a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenMethodConcurrencyCapTest.java b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenMethodConcurrencyCapTest.java index 00fbfdb07..3a330fdbb 100644 --- a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenMethodConcurrencyCapTest.java +++ b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenMethodConcurrencyCapTest.java @@ -140,9 +140,11 @@ public class AsyncPregenMethodConcurrencyCapTest { AtomicInteger warnings = new AtomicInteger(); ExecutorService executor = Executors.newSingleThreadExecutor(); try { - Future drain = executor.submit(() -> gate.awaitDrain( + Future drain = executor.submit(() -> gate.awaitDrain( 10L, TimeUnit.MILLISECONDS, + 5L, + TimeUnit.SECONDS, warnings::incrementAndGet )); @@ -152,7 +154,9 @@ public class AsyncPregenMethodConcurrencyCapTest { gate.release(); gate.release(); - assertFalse(drain.get(1L, TimeUnit.SECONDS)); + PregenAdmissionGate.Drain result = drain.get(1L, TimeUnit.SECONDS); + assertTrue(result.drained()); + assertFalse(result.interrupted()); assertTrue(warnings.get() > 0); assertEquals(2, gate.availablePermits()); } finally { diff --git a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenServerSaveOwnershipTest.java b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenServerSaveOwnershipTest.java index 74ee55644..542cf55b2 100644 --- a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenServerSaveOwnershipTest.java +++ b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenServerSaveOwnershipTest.java @@ -464,6 +464,8 @@ public class AsyncPregenServerSaveOwnershipTest { set("regionPending", pendingRegions); set("pendingEvictions", evictions); set("chunkIoExecutor", (Executor) Runnable::run); + set("generationFailure", new AtomicReference<>()); + set("inFlightRequests", new PregenInFlightRequests()); when(chunk.getX()).thenReturn(2); when(chunk.getZ()).thenReturn(3); when(binding.saveAndUnloadChunk(world, 2, 3)).thenReturn(true); diff --git a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenWorkerBoostTest.java b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenWorkerBoostTest.java index 4f1f36985..1e704ab49 100644 --- a/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenWorkerBoostTest.java +++ b/core/src/test/java/art/arcane/iris/world/pregen/AsyncPregenWorkerBoostTest.java @@ -113,7 +113,8 @@ public class AsyncPregenWorkerBoostTest { AsyncPregenMethod method = mock(AsyncPregenMethod.class, CALLS_REAL_METHODS); PregenAdmissionGate admission = mock(PregenAdmissionGate.class); IllegalStateException failure = new IllegalStateException("drain failed"); - when(admission.awaitDrain(anyLong(), any(TimeUnit.class), any(Runnable.class))).thenThrow(failure); + when(admission.awaitDrain(anyLong(), any(TimeUnit.class), anyLong(), any(TimeUnit.class), any(Runnable.class))) + .thenThrow(failure); set(method, "closing", new AtomicBoolean()); set(method, "holdsWorkerBoost", new AtomicBoolean(true)); set(method, "admission", admission); diff --git a/core/src/test/java/art/arcane/iris/world/pregen/PregenAdmissionGateTest.java b/core/src/test/java/art/arcane/iris/world/pregen/PregenAdmissionGateTest.java index 0bbb62400..69ac7e29b 100644 --- a/core/src/test/java/art/arcane/iris/world/pregen/PregenAdmissionGateTest.java +++ b/core/src/test/java/art/arcane/iris/world/pregen/PregenAdmissionGateTest.java @@ -12,6 +12,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -162,7 +163,8 @@ public class PregenAdmissionGateTest { AtomicInteger warnings = new AtomicInteger(); ExecutorService executor = Executors.newSingleThreadExecutor(); try { - Future drain = executor.submit(() -> gate.awaitDrain(5L, TimeUnit.MILLISECONDS, warnings::incrementAndGet)); + Future drain = executor.submit(() -> gate.awaitDrain( + 5L, TimeUnit.MILLISECONDS, 5L, TimeUnit.SECONDS, warnings::incrementAndGet)); while (warnings.get() < 2) { Thread.onSpinWait(); @@ -170,13 +172,46 @@ public class PregenAdmissionGateTest { gate.release(); gate.release(); - assertEquals(Boolean.FALSE, drain.get(5, TimeUnit.SECONDS)); + PregenAdmissionGate.Drain result = drain.get(5, TimeUnit.SECONDS); + assertTrue(result.drained()); + assertFalse(result.interrupted()); + assertEquals(0, result.outstanding()); assertEquals(2, gate.availablePermits()); } finally { executor.shutdownNow(); } } + @Test + public void drainGivesUpAtItsDeadlineWhilePermitsStayHeld() throws Exception { + PregenAdmissionGate gate = new PregenAdmissionGate(2, GENEROUS_BOUND_MS, System::currentTimeMillis); + assertNotNull(gate.admit(() -> false, () -> false)); + assertNotNull(gate.admit(() -> false, () -> false)); + AtomicInteger warnings = new AtomicInteger(); + + PregenAdmissionGate.Drain result = gate.awaitDrain( + 5L, TimeUnit.MILLISECONDS, 80L, TimeUnit.MILLISECONDS, warnings::incrementAndGet); + + assertFalse(result.drained()); + assertFalse(result.interrupted()); + assertEquals(2, result.outstanding()); + assertTrue(warnings.get() > 0); + assertEquals(0, gate.availablePermits()); + } + + @Test + public void drainReturnsAtOnceWhenNothingIsHeld() { + PregenAdmissionGate gate = new PregenAdmissionGate(2, GENEROUS_BOUND_MS, System::currentTimeMillis); + AtomicInteger warnings = new AtomicInteger(); + + PregenAdmissionGate.Drain result = gate.awaitDrain( + 5L, TimeUnit.MILLISECONDS, 5L, TimeUnit.SECONDS, warnings::incrementAndGet); + + assertTrue(result.drained()); + assertEquals(0, result.outstanding()); + assertEquals(0, warnings.get()); + } + @Test(expected = IllegalArgumentException.class) public void aGateWithoutPermitsIsRejected() { new PregenAdmissionGate(0, GENEROUS_BOUND_MS, System::currentTimeMillis); diff --git a/core/src/test/java/art/arcane/iris/world/pregen/PregenInFlightRequestsTest.java b/core/src/test/java/art/arcane/iris/world/pregen/PregenInFlightRequestsTest.java new file mode 100644 index 000000000..bd83f119c --- /dev/null +++ b/core/src/test/java/art/arcane/iris/world/pregen/PregenInFlightRequestsTest.java @@ -0,0 +1,62 @@ +package art.arcane.iris.world.pregen; + +import org.junit.Test; + +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +public class PregenInFlightRequestsTest { + @Test + public void settlingARequestReturnsItsListenerOnlyOnce() { + PregenInFlightRequests requests = new PregenInFlightRequests(); + PregenListener listener = mock(PregenListener.class); + long key = PregenInFlightRequests.key(48, -4); + + assertTrue(requests.add(key, listener)); + assertEquals(1, requests.size()); + assertSame(listener, requests.settle(key)); + assertNull(requests.settle(key)); + assertEquals(0, requests.size()); + } + + @Test + public void aDuplicateSubmissionKeepsTheOriginalRequest() { + PregenInFlightRequests requests = new PregenInFlightRequests(); + PregenListener first = mock(PregenListener.class); + PregenListener second = mock(PregenListener.class); + long key = PregenInFlightRequests.key(1, 2); + + assertTrue(requests.add(key, first)); + assertFalse(requests.add(key, second)); + assertSame(first, requests.settle(key)); + } + + @Test + public void drainingHandsBackEveryOutstandingRequestExactlyOnce() { + PregenInFlightRequests requests = new PregenInFlightRequests(); + PregenListener listener = mock(PregenListener.class); + requests.add(PregenInFlightRequests.key(0, 0), listener); + requests.add(PregenInFlightRequests.key(1, 0), listener); + requests.add(PregenInFlightRequests.key(-7, 9), listener); + + Map drained = requests.drain(); + + assertEquals(3, drained.size()); + assertEquals(0, requests.size()); + assertTrue(requests.drain().isEmpty()); + assertNull(requests.settle(PregenInFlightRequests.key(1, 0))); + } + + @Test + public void keysDistinguishNegativeCoordinates() { + assertTrue(PregenInFlightRequests.key(1, -1) != PregenInFlightRequests.key(-1, 1)); + assertTrue(PregenInFlightRequests.key(0, -1) != PregenInFlightRequests.key(-1, 0)); + assertEquals(PregenInFlightRequests.key(48, -4), PregenInFlightRequests.key(48, -4)); + } +}