From 5efef56fa5ffee72dbfa7282393bed1ba5f1f9fd Mon Sep 17 00:00:00 2001 From: Jaromir Hamala Date: Thu, 6 Aug 2026 15:07:01 +0200 Subject: [PATCH] test(qwp): make failed segment cleanup test deterministic When deleting an acknowledged store-and-forward segment fails, the client keeps the file so cleanup can be retried safely. The test failed deletion only once, allowing the background worker to delete the file before the test checked it. Keep deletion failing until those checks finish, then allow and verify cleanup. --- .../SegmentManagerUnlinkFailureTest.java | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java index b9ef46a5..b8c23901 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java @@ -44,6 +44,7 @@ import java.util.Arrays; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; public class SegmentManagerUnlinkFailureTest { @@ -123,12 +124,17 @@ public void testFailedUnlinkRetainsBookkeepingAndUsesSuccessorPath() throws Exce byte[] original = java.nio.file.Files.readAllBytes(Paths.get(failedPath)); FailingFilesFacade facade = new FailingFilesFacade(failedPath, null, null); + AtomicLong ticks = new AtomicLong(); try (SegmentManager manager = new SegmentManager( - segmentSize, TimeUnit.SECONDS.toNanos(60), segmentSize * 8, facade)) { + segmentSize, TimeUnit.SECONDS.toNanos(60), segmentSize * 8, facade, ticks::get)) { manager.register(ring, dir, watermark); manager.start(); Assert.assertTrue("manager never attempted the injected unlink", facade.removeAttempted.await(5, TimeUnit.SECONDS)); + ticks.set(TimeUnit.MINUTES.toNanos(1)); + manager.wakeWorker(); + Assert.assertTrue("manager never retried the injected unlink", + facade.removeRetried.await(5, TimeUnit.SECONDS)); Assert.assertEquals("failed unlink must retain conservative registered bytes", ring.totalSegmentBytes(), readTotalBytes(manager)); } @@ -157,6 +163,7 @@ public void testFailedUnlinkRetainsBookkeepingAndUsesSuccessorPath() throws Exce Assert.assertTrue("successor write overwrote the failed-unlink segment", Arrays.equals(original, java.nio.file.Files.readAllBytes(Paths.get(failedPath)))); + facade.allowUnlink(); try (SegmentManager retryManager = new SegmentManager( segmentSize, TimeUnit.SECONDS.toNanos(60), segmentSize * 8, facade)) { retryManager.register(ring, dir, watermark); @@ -243,11 +250,13 @@ private static final class FailingFilesFacade implements FilesFacade { private final String partialLowerName; private final String unlinkFailurePath; private final CountDownLatch removeAttempted = new CountDownLatch(1); + private final CountDownLatch removeRetried = new CountDownLatch(1); private int openCleanCalls; private boolean partialFindClosed; private long partialFindNamePtr; private boolean partialLowerObserved; - private int unlinkFailuresRemaining = 1; + private int unlinkFailureCount; + private volatile boolean unlinkFailureEnabled = true; private FailingFilesFacade( String unlinkFailurePath, @@ -259,6 +268,10 @@ private FailingFilesFacade( this.partialLowerName = partialLowerName; } + private void allowUnlink() { + unlinkFailureEnabled = false; + } + @Override public boolean allocate(int fd, long size) { return INSTANCE.allocate(fd, size); } @Override @@ -333,8 +346,14 @@ public long read(int fd, long addr, long len, long offset) { } @Override public boolean remove(String path) { - if (path.equals(unlinkFailurePath) && unlinkFailuresRemaining > 0) { - unlinkFailuresRemaining--; + // Keep the injected fault active until the test explicitly releases + // it. SegmentManager automatically retries failed trims, and its + // poll park may return early, so a one-shot failure can recover + // before the test observes the retained bookkeeping state. + if (unlinkFailureEnabled && path.equals(unlinkFailurePath)) { + if (++unlinkFailureCount > 1) { + removeRetried.countDown(); + } removeAttempted.countDown(); return false; }