Skip to content

Commit d3b6147

Browse files
committed
test(qwp): pin slot retention on the owned-manager close path
Every production CursorSendEngine (Sender.build, BackgroundDrainer, QwpWebSocketSender.connect) owns its SegmentManager, so close() takes the manager.close() + isWorkerReaped() branch - yet all deterministic retention tests exercised the test-only shared-manager branch (awaitRingQuiescence). A regression confined to the owned path - reporting quiescence unconditionally, or isWorkerReaped() returning true while the worker is alive - would have gone green through the whole suite and silently reintroduced the SF-data-loss hazard on the only path production runs. testOwnedEngineCloseRetainsSlotWhileWorkerIsMidServicePass builds the production shape (2-arg ctor, private owned manager), waits for the initial hot-spare install so the park hook can neither be missed nor fire early, rotates onto the spare to force the worker back into an install pass, parks it there, and drives close() with a 50 ms join budget. It asserts the incomplete close stays observable (isCloseCompleted() == false), the slot flock is retained (SlotLock.acquire throws), and a retried close after worker release completes the full cleanup and frees the slot. Mutation-verified red on all three reverts: - owned branch forcing workerQuiescent = true (only this test catches it) - finally gate reverted to unconditional slotLock.close() - SegmentManager.isWorkerReaped() returning true while the worker is alive (previously zero coverage anywhere)
1 parent d976fd5 commit d3b6147

1 file changed

Lines changed: 123 additions & 0 deletions

File tree

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineSlotReacquisitionTest.java

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@
2727
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
2828
import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment;
2929
import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager;
30+
import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentRing;
3031
import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock;
3132
import io.questdb.client.std.Files;
33+
import io.questdb.client.std.MemoryTag;
34+
import io.questdb.client.std.Unsafe;
3235
import io.questdb.client.test.tools.TestUtils;
3336
import org.junit.After;
3437
import org.junit.Assert;
@@ -179,6 +182,120 @@ public void testCloseRetainsSlotWhileWorkerIsMidServicePass() throws Exception {
179182
});
180183
}
181184

185+
/**
186+
* Owned-manager twin of {@link #testCloseRetainsSlotWhileWorkerIsMidServicePass}:
187+
* the ONLY construction shape production uses (Sender.build, BackgroundDrainer,
188+
* QwpWebSocketSender.connect all own their manager). The owned close path does
189+
* not run the per-ring barrier at all — it relies on {@code manager.close()}'s
190+
* bounded join and the {@code isWorkerReaped()} check. If that check regressed
191+
* to report quiescence unconditionally (or {@code isWorkerReaped()} itself
192+
* returned true while the worker is alive), close() would release the slot
193+
* lock mid service pass and the shared-manager tests would stay green — this
194+
* test is the red gate for the production path.
195+
* <p>
196+
* Determinism: the owned manager starts inside the engine ctor (1 ms poll),
197+
* so its first spare-install pass races test setup. We first wait until the
198+
* initial hot spare is installed — after that the worker cannot enter another
199+
* install pass until a rotation consumes the spare, so the park hook installed
200+
* afterwards can neither be missed nor fire early. Two appends then fill the
201+
* active segment and rotate onto the spare; the worker's next poll tick
202+
* re-enters the install pass and parks in the hook.
203+
*/
204+
@Test(timeout = 30_000L)
205+
public void testOwnedEngineCloseRetainsSlotWhileWorkerIsMidServicePass() throws Exception {
206+
TestUtils.assertMemoryLeak(() -> {
207+
final int payloadLen = 32;
208+
long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + payloadLen);
209+
String slot = tmpDir + "/owned-parked-slot";
210+
CountDownLatch workerBlocked = new CountDownLatch(1);
211+
CountDownLatch releaseWorker = new CountDownLatch(1);
212+
AtomicBoolean fired = new AtomicBoolean();
213+
AtomicReference<Throwable> hookErr = new AtomicReference<>();
214+
// Production shape: private, owned manager (ownsManager=true).
215+
CursorSendEngine engine = new CursorSendEngine(slot, segSize);
216+
SegmentManager manager = readManager(engine);
217+
long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT);
218+
try {
219+
// Phase 1: let the worker finish the initial spare install so
220+
// the hook below can only fire on the rotation-triggered pass.
221+
SegmentRing ring = readRing(engine);
222+
long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
223+
while (ring.needsHotSpare()) {
224+
if (System.nanoTime() > deadlineNs) {
225+
throw new AssertionError("manager worker never installed the initial hot spare");
226+
}
227+
Thread.sleep(1);
228+
}
229+
manager.setBeforeInstallSyncHook(() -> {
230+
if (!fired.compareAndSet(false, true)) return;
231+
workerBlocked.countDown();
232+
try {
233+
if (!releaseWorker.await(20, TimeUnit.SECONDS)) {
234+
hookErr.compareAndSet(null,
235+
new AssertionError("timed out waiting for test to release worker"));
236+
}
237+
} catch (Throwable t) {
238+
hookErr.compareAndSet(null, t);
239+
}
240+
});
241+
242+
// Phase 2: one frame fills the active segment exactly; the
243+
// second forces rotation onto the spare. needsHotSpare() is
244+
// true again, so the worker's next tick parks in the hook.
245+
Unsafe.getUnsafe().putLong(buf, 0L);
246+
Assert.assertEquals(0L, engine.appendBlocking(buf, payloadLen));
247+
Assert.assertEquals(1L, engine.appendBlocking(buf, payloadLen));
248+
Assert.assertTrue("worker never re-entered a spare-install pass",
249+
workerBlocked.await(5, TimeUnit.SECONDS));
250+
251+
// Phase 3: owned close with the worker provably mid service
252+
// pass. manager.close()'s 50 ms join times out, the worker is
253+
// not reaped, and close() must retain every worker-reachable
254+
// resource — above all the slot flock.
255+
manager.setWorkerJoinTimeoutMillis(50L);
256+
engine.close();
257+
Assert.assertFalse("incomplete owned close must remain observable to the owner",
258+
engine.isCloseCompleted());
259+
try {
260+
SlotLock probe = SlotLock.acquire(slot);
261+
probe.close();
262+
Assert.fail("owned engine.close() released the slot lock while its manager "
263+
+ "worker was still mid service pass — a replacement engine could "
264+
+ "acquire the slot and have its segment files unlinked by the "
265+
+ "stale worker (the production SF-data-loss hazard)");
266+
} catch (Exception expected) {
267+
// good — slot retained.
268+
}
269+
270+
// Phase 4: release the worker (it abandons the spare: the ring
271+
// was deregistered by the close attempt) and retry. The join
272+
// now reaps the worker and the full cleanup must complete.
273+
releaseWorker.countDown();
274+
manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60));
275+
engine.close();
276+
Assert.assertTrue("retried owned close must report complete cleanup",
277+
engine.isCloseCompleted());
278+
try (SlotLock probe = SlotLock.acquire(slot)) {
279+
Assert.assertNotNull("slot must be acquirable after a completed close", probe);
280+
} catch (Exception e) {
281+
throw new AssertionError("retried owned close() did not release the slot lock", e);
282+
}
283+
if (hookErr.get() != null) {
284+
throw new AssertionError("install hook failed", hookErr.get());
285+
}
286+
} finally {
287+
Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT);
288+
manager.setBeforeInstallSyncHook(null);
289+
releaseWorker.countDown();
290+
manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60));
291+
try {
292+
engine.close();
293+
} catch (Throwable ignored) {
294+
}
295+
}
296+
});
297+
}
298+
182299
/**
183300
* An engine that owns its manager must use the whole-manager stop/join as
184301
* its only quiescence barrier. Calling the per-ring barrier first would
@@ -230,6 +347,12 @@ private static SegmentManager readManager(CursorSendEngine engine) throws Except
230347
return (SegmentManager) field.get(engine);
231348
}
232349

350+
private static SegmentRing readRing(CursorSendEngine engine) throws Exception {
351+
Field field = CursorSendEngine.class.getDeclaredField("ring");
352+
field.setAccessible(true);
353+
return (SegmentRing) field.get(engine);
354+
}
355+
233356
private static void rmDirRecursive(String dir) {
234357
if (!Files.exists(dir)) return;
235358
long find = Files.findFirst(dir);

0 commit comments

Comments
 (0)