Skip to content

Commit 16df6f6

Browse files
puzpuzpuzclaude
andcommitted
test(qwp): dedupe SF tmp-dir fixtures; name the sparse-tail cause in the FSN-gap error
Follow-up on the PR #64 review minors. m3: MmapSegmentTest and MmapSegmentRecoveryFaultTest carried verbatim copies of the native tmp-dir setUp/tearDown (~28 lines each). Hoist them into TestUtils.createTmpDir / removeTmpDir so both classes and any future SF cursor test share one implementation. m5: the FSN-gap MmapSegmentException fired on a sealed segment that under-recovered (a sparse/unbacked or media-truncated tail), but its message and comment blamed only "partial-write/manual-deletion". Name the truncated-tail cause and ask operators to check disk health, matching the diagnostics-honesty wording already added to the scanFrames WARN. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2721f39 commit 16df6f6

4 files changed

Lines changed: 57 additions & 51 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,12 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
265265
// pivot would degrade back to O(N²) on exactly that common case.
266266
sortByBaseSeq(opened, 0, opened.size());
267267
// Sanity: the recovered segments must form a contiguous FSN range.
268-
// Detect gaps so a partial-write/manual-deletion mishap doesn't
269-
// silently produce duplicate or missing FSNs after recovery.
268+
// Detect gaps so they don't silently produce duplicate or missing
269+
// FSNs after recovery. A gap means a segment went missing (a
270+
// manual deletion) or a sealed segment under-recovered -- its tail
271+
// was cut short by a sparse/unbacked page or a mid-file media error
272+
// (bad sector), the same class of fault scanFrames tolerates on the
273+
// active segment but which corrupts the range on a sealed one.
270274
for (int i = 1, n = opened.size(); i < n; i++) {
271275
MmapSegment prev = opened.get(i - 1);
272276
MmapSegment curr = opened.get(i);
@@ -276,7 +280,10 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
276280
"FSN gap in recovered segments: prev baseSeq=" + prev.baseSeq()
277281
+ " frameCount=" + prev.frameCount()
278282
+ " expected next baseSeq=" + expected
279-
+ " but got " + curr.baseSeq());
283+
+ " but got " + curr.baseSeq()
284+
+ " -- a segment was deleted, or a sealed segment's tail was"
285+
+ " truncated (sparse/unbacked page or disk media error);"
286+
+ " check disk health");
280287
}
281288
}
282289
// The newest segment becomes the active. Even if it's full, that's OK:

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

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@
3434
import org.junit.Before;
3535
import org.junit.Test;
3636

37-
import java.nio.file.Paths;
38-
3937
import static org.junit.Assert.assertEquals;
4038
import static org.junit.Assert.assertTrue;
4139
import static org.junit.Assert.fail;
@@ -81,32 +79,12 @@ public class MmapSegmentRecoveryFaultTest {
8179

8280
@Before
8381
public void setUp() {
84-
tmpDir = Paths.get(System.getProperty("java.io.tmpdir"),
85-
"qdb-mmap-recover-" + System.nanoTime()).toString();
86-
assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT));
82+
tmpDir = TestUtils.createTmpDir("qdb-mmap-recover-");
8783
}
8884

8985
@After
9086
public void tearDown() {
91-
if (tmpDir == null) {
92-
return;
93-
}
94-
long find = Files.findFirst(tmpDir);
95-
if (find > 0) {
96-
try {
97-
int rc = 1;
98-
while (rc > 0) {
99-
String name = Files.utf8ToString(Files.findName(find));
100-
if (name != null && !".".equals(name) && !"..".equals(name)) {
101-
Files.remove(tmpDir + "/" + name);
102-
}
103-
rc = Files.findNext(find);
104-
}
105-
} finally {
106-
Files.findClose(find);
107-
}
108-
}
109-
Files.remove(tmpDir);
87+
TestUtils.removeTmpDir(tmpDir);
11088
}
11189

11290
/**

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

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@
3535
import org.junit.Before;
3636
import org.junit.Test;
3737

38-
import java.nio.file.Paths;
39-
4038
import static org.junit.Assert.assertEquals;
4139
import static org.junit.Assert.assertFalse;
4240
import static org.junit.Assert.assertNotEquals;
@@ -49,32 +47,12 @@ public class MmapSegmentTest {
4947

5048
@Before
5149
public void setUp() {
52-
tmpDir = Paths.get(System.getProperty("java.io.tmpdir"),
53-
"qdb-mmap-seg-" + System.nanoTime()).toString();
54-
assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT));
50+
tmpDir = TestUtils.createTmpDir("qdb-mmap-seg-");
5551
}
5652

5753
@After
5854
public void tearDown() {
59-
if (tmpDir == null) {
60-
return;
61-
}
62-
long find = Files.findFirst(tmpDir);
63-
if (find > 0) {
64-
try {
65-
int rc = 1;
66-
while (rc > 0) {
67-
String name = Files.utf8ToString(Files.findName(find));
68-
if (name != null && !".".equals(name) && !"..".equals(name)) {
69-
Files.remove(tmpDir + "/" + name);
70-
}
71-
rc = Files.findNext(find);
72-
}
73-
} finally {
74-
Files.findClose(find);
75-
}
76-
}
77-
Files.remove(tmpDir);
55+
TestUtils.removeTmpDir(tmpDir);
7856
}
7957

8058
@Test

core/src/test/java/io/questdb/client/test/tools/TestUtils.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
package io.questdb.client.test.tools;
2626

27+
import io.questdb.client.std.Files;
2728
import io.questdb.client.std.MemoryTag;
2829
import io.questdb.client.std.QuietCloseable;
2930
import io.questdb.client.std.Rnd;
@@ -139,6 +140,18 @@ public static long beHexToLong(String hex) {
139140
return Long.parseLong(reverseBeHex(hex), 16);
140141
}
141142

143+
/**
144+
* Creates a unique temp directory under {@code java.io.tmpdir}, named
145+
* {@code prefix + <nanoTime>}, and returns its path. Pair with
146+
* {@link #removeTmpDir(String)} in {@code tearDown}.
147+
*/
148+
public static String createTmpDir(String prefix) {
149+
String dir = Paths.get(System.getProperty("java.io.tmpdir"),
150+
prefix + System.nanoTime()).toString();
151+
Assert.assertEquals(0, Files.mkdir(dir, Files.DIR_MODE_DEFAULT));
152+
return dir;
153+
}
154+
142155
@NotNull
143156
public static Rnd generateRandom(Logger log) {
144157
return generateRandom(log, System.nanoTime(), System.currentTimeMillis());
@@ -172,6 +185,36 @@ public static String ipv4ToString(int ip) {
172185
return ((ip >> 24) & 0xff) + "." + ((ip >> 16) & 0xff) + "." + ((ip >> 8) & 0xff) + "." + (ip & 0xff);
173186
}
174187

188+
/**
189+
* Flat (non-recursive) cleanup for a directory created by
190+
* {@link #createTmpDir(String)}: removes every entry in {@code tmpDir}
191+
* (the SF cursor tests only write flat {@code .sfa}/{@code .corrupt}
192+
* files) and then the directory itself. A {@code null} argument is a
193+
* no-op, so it is safe to call from {@code tearDown} before {@code setUp}
194+
* ran.
195+
*/
196+
public static void removeTmpDir(String tmpDir) {
197+
if (tmpDir == null) {
198+
return;
199+
}
200+
long find = Files.findFirst(tmpDir);
201+
if (find > 0) {
202+
try {
203+
int rc = 1;
204+
while (rc > 0) {
205+
String name = Files.utf8ToString(Files.findName(find));
206+
if (name != null && !".".equals(name) && !"..".equals(name)) {
207+
Files.remove(tmpDir + "/" + name);
208+
}
209+
rc = Files.findNext(find);
210+
}
211+
} finally {
212+
Files.findClose(find);
213+
}
214+
}
215+
Files.remove(tmpDir);
216+
}
217+
175218
/**
176219
* Java 8 stand-in for {@code String.repeat(int)} (added in Java 11).
177220
*/

0 commit comments

Comments
 (0)