Skip to content

Commit d89f14f

Browse files
glasstigerclaude
andcommitted
Make the delta-dict tests deterministic and leak-checked
Two robustness fixes to the delta symbol-dictionary tests. Deterministic synchronization (replaces fixed sleeps): - DeltaDictCatchUpTest waited a fixed 200 ms for the server to close connection 1 before sending batch 2. On a loaded machine that could under-wait and let batch 2 race into connection 1's pre-close window, changing which connection the catch-up lands on. The handler now sets a conn1Closed flag after it closes the socket, and the test waits on that. - DeltaDictRecoveryTest's torn-dictionary test slept a fixed 1 s to let the replay guard fire before close(). It now polls flush() for the latched terminal (close() remains the fallback), so it captures the terminal as soon as it fires -- the run dropped from ~1 s to ~0.3 s. Leak checks: the Sender-based tests allocate native memory (the send-loop mirror, persisted-dict buffers, segment mmaps) but were not wrapped in assertMemoryLeak, unlike the rest of the suite. Wrap all eight methods across the three classes; every one is balanced (they already cleaned up via try-with-resources -- the wrapper now guards against future leaks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0f9d88f commit d89f14f

3 files changed

Lines changed: 367 additions & 318 deletions

File tree

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/DeltaDictCatchUpTest.java

Lines changed: 122 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import java.util.concurrent.atomic.AtomicInteger;
4343
import java.util.concurrent.atomic.AtomicLong;
4444

45+
import static io.questdb.client.test.tools.TestUtils.assertMemoryLeak;
46+
4547
/**
4648
* Verifies the delta symbol-dictionary catch-up on reconnect (memory-mode).
4749
* <p>
@@ -61,37 +63,40 @@ public void testReconnectCatchUpRebuildsDictionary() throws Exception {
6163
// sender reconnects. Connection 2 (fresh, empty dict): send "beta" (id 1).
6264
// Without catch-up, connection 2's first data frame would carry
6365
// deltaStart=1 and the fresh server would never learn id 0.
64-
CatchUpHandler handler = new CatchUpHandler();
65-
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
66-
int port = server.getPort();
67-
server.start();
68-
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
69-
70-
try (Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";")) {
71-
sender.table("t").symbol("s", "alpha").longColumn("v", 1L).atNow();
72-
sender.flush();
73-
waitFor(() -> handler.dictFor(1).size() >= 1, 5_000);
74-
75-
// Let the I/O loop observe the server-side close before the next
76-
// batch, so batch 2 is what drives the reconnect + catch-up.
77-
Thread.sleep(200);
78-
79-
sender.table("t").symbol("s", "beta").longColumn("v", 2L).atNow();
80-
sender.flush();
81-
waitFor(() -> handler.connectionsAccepted.get() >= 2
82-
&& handler.dictFor(2).size() >= 2, 5_000);
83-
}
66+
assertMemoryLeak(() -> {
67+
CatchUpHandler handler = new CatchUpHandler();
68+
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
69+
int port = server.getPort();
70+
server.start();
71+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
72+
73+
try (Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";")) {
74+
sender.table("t").symbol("s", "alpha").longColumn("v", 1L).atNow();
75+
sender.flush();
76+
waitFor(() -> handler.dictFor(1).size() >= 1, 5_000);
8477

85-
// The fresh (2nd) connection's dictionary, rebuilt purely from the
86-
// frames it received, must hold both symbols contiguously with no
87-
// null gap -- exactly what the catch-up frame guarantees.
88-
List<String> conn2 = handler.dictFor(2);
89-
Assert.assertTrue("2nd connection saw a catch-up frame with 0 tables",
90-
handler.sawZeroTableFrameOnConn2);
91-
Assert.assertEquals("2nd connection dictionary size", 2, conn2.size());
92-
Assert.assertEquals("alpha", conn2.get(0));
93-
Assert.assertEquals("beta", conn2.get(1));
94-
}
78+
// Wait until the server has actually closed connection 1 before
79+
// sending batch 2, so batch 2 cannot race into connection 1 and
80+
// must drive the reconnect + catch-up.
81+
waitFor(() -> handler.conn1Closed, 5_000);
82+
83+
sender.table("t").symbol("s", "beta").longColumn("v", 2L).atNow();
84+
sender.flush();
85+
waitFor(() -> handler.connectionsAccepted.get() >= 2
86+
&& handler.dictFor(2).size() >= 2, 5_000);
87+
}
88+
89+
// The fresh (2nd) connection's dictionary, rebuilt purely from the
90+
// frames it received, must hold both symbols contiguously with no
91+
// null gap -- exactly what the catch-up frame guarantees.
92+
List<String> conn2 = handler.dictFor(2);
93+
Assert.assertTrue("2nd connection saw a catch-up frame with 0 tables",
94+
handler.sawZeroTableFrameOnConn2);
95+
Assert.assertEquals("2nd connection dictionary size", 2, conn2.size());
96+
Assert.assertEquals("alpha", conn2.get(0));
97+
Assert.assertEquals("beta", conn2.get(1));
98+
}
99+
});
95100
}
96101

97102
@Test
@@ -109,48 +114,50 @@ public void testCatchUpEntryTooLargeForCapFailsTerminally() throws Exception {
109114
// reconnect's catch-up cannot re-ship the symbol. (One fixed cap can't do
110115
// this: the client refuses to SEND a single-table frame over the cap, and
111116
// that data frame is always larger than the bare catch-up entry.)
112-
CapShrinkHandler handler = new CapShrinkHandler();
113-
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
114-
handler.setServer(server);
115-
int port = server.getPort();
116-
server.start();
117-
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
118-
119-
String bigSymbol = TestUtils.repeat("x", 200); // ~202-byte dict entry
120-
LineSenderException terminal = null;
121-
Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";");
122-
try {
123-
sender.table("t").symbol("s", bigSymbol).longColumn("v", 1L).atNow();
124-
sender.flush();
125-
// The terminal latches on the I/O thread once the reconnect's
126-
// catch-up hits the oversized entry; it surfaces to the producer
127-
// on a subsequent flush. Poll a bounded time for it. The polling
128-
// rows use a small symbol that fits the shrunk cap, so the
129-
// producer-side cap check never fires and flush() surfaces the
130-
// I/O thread's catch-up terminal via checkError.
131-
long deadline = System.currentTimeMillis() + 10_000;
132-
while (System.currentTimeMillis() < deadline && terminal == null) {
117+
assertMemoryLeak(() -> {
118+
CapShrinkHandler handler = new CapShrinkHandler();
119+
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
120+
handler.setServer(server);
121+
int port = server.getPort();
122+
server.start();
123+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
124+
125+
String bigSymbol = TestUtils.repeat("x", 200); // ~202-byte dict entry
126+
LineSenderException terminal = null;
127+
Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";");
128+
try {
129+
sender.table("t").symbol("s", bigSymbol).longColumn("v", 1L).atNow();
130+
sender.flush();
131+
// The terminal latches on the I/O thread once the reconnect's
132+
// catch-up hits the oversized entry; it surfaces to the producer
133+
// on a subsequent flush. Poll a bounded time for it. The polling
134+
// rows use a small symbol that fits the shrunk cap, so the
135+
// producer-side cap check never fires and flush() surfaces the
136+
// I/O thread's catch-up terminal via checkError.
137+
long deadline = System.currentTimeMillis() + 10_000;
138+
while (System.currentTimeMillis() < deadline && terminal == null) {
139+
try {
140+
sender.table("t").symbol("s", "y").longColumn("v", 2L).atNow();
141+
sender.flush();
142+
Thread.sleep(20);
143+
} catch (LineSenderException e) {
144+
terminal = e;
145+
}
146+
}
147+
} finally {
133148
try {
134-
sender.table("t").symbol("s", "y").longColumn("v", 2L).atNow();
135-
sender.flush();
136-
Thread.sleep(20);
149+
sender.close();
137150
} catch (LineSenderException e) {
138-
terminal = e;
139-
}
140-
}
141-
} finally {
142-
try {
143-
sender.close();
144-
} catch (LineSenderException e) {
145-
if (terminal == null) {
146-
terminal = e;
151+
if (terminal == null) {
152+
terminal = e;
153+
}
147154
}
148155
}
156+
Assert.assertNotNull("an oversized catch-up entry must surface a terminal", terminal);
157+
Assert.assertTrue("terminal must come from the catch-up path, got: " + terminal.getMessage(),
158+
terminal.getMessage().contains("during catch-up"));
149159
}
150-
Assert.assertNotNull("an oversized catch-up entry must surface a terminal", terminal);
151-
Assert.assertTrue("terminal must come from the catch-up path, got: " + terminal.getMessage(),
152-
terminal.getMessage().contains("during catch-up"));
153-
}
160+
});
154161
}
155162

156163
@Test
@@ -163,48 +170,50 @@ public void testReconnectCatchUpSplitsLargeDictionaryAcrossFrames() throws Excep
163170
// into several contiguous zero-table frames that the fresh server stitches
164171
// back into a complete, gap-free dictionary.
165172
final int symbolCount = 40;
166-
SplitCatchUpHandler handler = new SplitCatchUpHandler(symbolCount);
167-
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
168-
server.setAdvertisedMaxBatchSize(160); // small cap forces the catch-up to split
169-
int port = server.getPort();
170-
server.start();
171-
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
172-
173-
try (Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";")) {
174-
// One row per flush so each frame stays under the 160-byte cap; the
175-
// sent dictionary still accumulates all 40 symbols on connection 1.
176-
for (int i = 0; i < symbolCount; i++) {
177-
sender.table("t").symbol("s", symbolName(i)).longColumn("v", i).atNow();
173+
assertMemoryLeak(() -> {
174+
SplitCatchUpHandler handler = new SplitCatchUpHandler(symbolCount);
175+
try (TestWebSocketServer server = new TestWebSocketServer(handler)) {
176+
server.setAdvertisedMaxBatchSize(160); // small cap forces the catch-up to split
177+
int port = server.getPort();
178+
server.start();
179+
Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS));
180+
181+
try (Sender sender = Sender.fromConfig("ws::addr=localhost:" + port + ";")) {
182+
// One row per flush so each frame stays under the 160-byte cap; the
183+
// sent dictionary still accumulates all 40 symbols on connection 1.
184+
for (int i = 0; i < symbolCount; i++) {
185+
sender.table("t").symbol("s", symbolName(i)).longColumn("v", i).atNow();
186+
sender.flush();
187+
}
188+
// Wait until connection 1 has learned every symbol, so the sender's
189+
// sent-dictionary mirror (the catch-up source) holds all of them.
190+
waitFor(() -> handler.dictFor(1).size() >= symbolCount, 10_000);
191+
192+
// Wait until the server has actually closed connection 1 before
193+
// sending batch 2, so batch 2 drives the reconnect + split catch-up.
194+
waitFor(() -> handler.conn1Closed, 5_000);
195+
196+
sender.table("t").symbol("s", symbolName(symbolCount)).longColumn("v", symbolCount).atNow();
178197
sender.flush();
198+
waitFor(() -> handler.connectionsAccepted.get() >= 2
199+
&& handler.dictFor(2).size() >= symbolCount + 1, 10_000);
179200
}
180-
// Wait until connection 1 has learned every symbol, so the sender's
181-
// sent-dictionary mirror (the catch-up source) holds all of them.
182-
waitFor(() -> handler.dictFor(1).size() >= symbolCount, 10_000);
183-
184-
// Let the I/O loop observe the server-side close before the next
185-
// batch, so batch 2 drives the reconnect + split catch-up.
186-
Thread.sleep(200);
187-
188-
sender.table("t").symbol("s", symbolName(symbolCount)).longColumn("v", symbolCount).atNow();
189-
sender.flush();
190-
waitFor(() -> handler.connectionsAccepted.get() >= 2
191-
&& handler.dictFor(2).size() >= symbolCount + 1, 10_000);
192-
}
193201

194-
// Connection 2's dictionary, rebuilt purely from the frames it received,
195-
// must hold every symbol contiguously with no null gap -- the split
196-
// catch-up frames reassemble exactly.
197-
List<String> conn2 = handler.dictFor(2);
198-
Assert.assertEquals("2nd connection dictionary size", symbolCount + 1, conn2.size());
199-
for (int i = 0; i <= symbolCount; i++) {
200-
Assert.assertEquals("symbol at id " + i, symbolName(i), conn2.get(i));
202+
// Connection 2's dictionary, rebuilt purely from the frames it received,
203+
// must hold every symbol contiguously with no null gap -- the split
204+
// catch-up frames reassemble exactly.
205+
List<String> conn2 = handler.dictFor(2);
206+
Assert.assertEquals("2nd connection dictionary size", symbolCount + 1, conn2.size());
207+
for (int i = 0; i <= symbolCount; i++) {
208+
Assert.assertEquals("symbol at id " + i, symbolName(i), conn2.get(i));
209+
}
210+
// The catch-up had to span more than one zero-table frame to stay under
211+
// the advertised cap -- that split is the behaviour under test.
212+
Assert.assertTrue("catch-up split into multiple frames (saw "
213+
+ handler.zeroTableFramesOnConn2 + ")",
214+
handler.zeroTableFramesOnConn2 >= 2);
201215
}
202-
// The catch-up had to span more than one zero-table frame to stay under
203-
// the advertised cap -- that split is the behaviour under test.
204-
Assert.assertTrue("catch-up split into multiple frames (saw "
205-
+ handler.zeroTableFramesOnConn2 + ")",
206-
handler.zeroTableFramesOnConn2 >= 2);
207-
}
216+
});
208217
}
209218

210219
private static String symbolName(int i) {
@@ -252,6 +261,10 @@ private interface BoolCondition {
252261
*/
253262
private static class CatchUpHandler implements TestWebSocketServer.WebSocketServerHandler {
254263
final AtomicInteger connectionsAccepted = new AtomicInteger();
264+
// Set once the server has closed connection 1. A test waits on this
265+
// (rather than a fixed sleep) before sending batch 2, so batch 2 cannot
266+
// race into connection 1's pre-close window and must land on the reconnect.
267+
volatile boolean conn1Closed;
255268
volatile boolean sawZeroTableFrameOnConn2;
256269
private final List<List<String>> dictsByConn = new CopyOnWriteArrayList<>();
257270
private TestWebSocketServer.ClientHandler currentClient;
@@ -284,6 +297,7 @@ public synchronized void onBinaryMessage(TestWebSocketServer.ClientHandler clien
284297
if (connNumber == 1) {
285298
Thread.sleep(50);
286299
client.close();
300+
conn1Closed = true;
287301
}
288302
} catch (IOException | InterruptedException e) {
289303
Thread.currentThread().interrupt();
@@ -377,6 +391,8 @@ public synchronized void onBinaryMessage(TestWebSocketServer.ClientHandler clien
377391
*/
378392
private static class SplitCatchUpHandler implements TestWebSocketServer.WebSocketServerHandler {
379393
final AtomicInteger connectionsAccepted = new AtomicInteger();
394+
// Set once the server has closed connection 1 (see CatchUpHandler.conn1Closed).
395+
volatile boolean conn1Closed;
380396
volatile int zeroTableFramesOnConn2;
381397
private final List<List<String>> dictsByConn = new CopyOnWriteArrayList<>();
382398
private final int dropConn1AtDictSize;
@@ -417,6 +433,7 @@ public synchronized void onBinaryMessage(TestWebSocketServer.ClientHandler clien
417433
conn1Dropped = true;
418434
Thread.sleep(50);
419435
client.close();
436+
conn1Closed = true;
420437
}
421438
} catch (IOException | InterruptedException e) {
422439
Thread.currentThread().interrupt();

0 commit comments

Comments
 (0)