Skip to content

Commit 80ad4ca

Browse files
committed
fix(qwp): harden orphan recovery and metadata crash safety
1 parent 71cfbe2 commit 80ad4ca

18 files changed

Lines changed: 808 additions & 75 deletions

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import io.questdb.client.cutlass.line.LineSenderException;
4040
import io.questdb.client.cutlass.line.array.DoubleArray;
4141
import io.questdb.client.cutlass.line.array.LongArray;
42+
import io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer;
4243
import io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerListener;
4344
import io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerPool;
4445
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
@@ -2330,7 +2331,7 @@ public synchronized void setDrainerListener(BackgroundDrainerListener listener)
23302331
pool.setListener(listener);
23312332
// ...and direct re-assignment for the ones already running (the
23322333
// pool listener is only applied at submit time, never after).
2333-
ObjList<io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer> live =
2334+
ObjList<BackgroundDrainer> live =
23342335
pool.snapshot();
23352336
for (int i = 0, n = live.size(); i < n; i++) {
23362337
live.getQuick(i).setListener(listener);
@@ -2473,16 +2474,16 @@ public synchronized void startOrphanDrainers(
24732474
// (the factory needs the drainer, the drainer's constructor
24742475
// needs the factory); the ref write happens-before the drainer
24752476
// runs because submit() publishes the task afterwards.
2476-
final io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer[] ref =
2477-
new io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer[1];
2477+
final BackgroundDrainer[] ref =
2478+
new BackgroundDrainer[1];
24782479
ReconnectSupplier factory = new ReconnectSupplier(
24792480
() -> {
2480-
io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer d = ref[0];
2481+
BackgroundDrainer d = ref[0];
24812482
return d != null && d.isStopRequested();
24822483
},
24832484
"drainer stop requested during connect");
2484-
io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer drainer =
2485-
new io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer(
2485+
BackgroundDrainer drainer =
2486+
new BackgroundDrainer(
24862487
slot, segmentSizeBytes, sfMaxTotalBytes,
24872488
syncIntervalNanos,
24882489
factory,

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@
4545
* "everything &lt;= N is durable"), so a single monotonic watermark suffices;
4646
* no per-frame bitmap is needed.
4747
* <p>
48-
* <b>Layout</b> (128 bytes, little-endian, mmap'd for the engine's lifetime):
49-
* two independently CRC-protected 64-byte records. Each record contains:
48+
* <b>Layout</b> (8192 bytes, little-endian, mmap'd for the engine's lifetime):
49+
* two independently CRC-protected 64-byte records at offsets 0 and 4096.
50+
* Placing each record at the start of a separate 4 KiB slot prevents one
51+
* aligned 512-byte or 4 KiB sector tear from damaging both. Each record contains:
5052
* <pre>
5153
* offset 0: u32 magic = 'AKW1'
5254
* offset 4: u32 version = 1
@@ -84,7 +86,7 @@ public final class AckWatermark implements QuietCloseable {
8486
* directory enumerators that filter by extension skip it automatically.
8587
*/
8688
public static final String FILE_NAME = ".ack-watermark";
87-
public static final int FILE_SIZE = 128;
89+
public static final int FILE_SIZE = 8 * 1024;
8890
/**
8991
* Sentinel returned by {@link #read()} when neither watermark record is
9092
* valid.
@@ -96,6 +98,7 @@ public final class AckWatermark implements QuietCloseable {
9698
private static final Logger LOG = LoggerFactory.getLogger(AckWatermark.class);
9799
private static final int MAGIC_OFFSET = 0;
98100
private static final int RECORD_SIZE = 64;
101+
private static final int RECORD_SLOT_SIZE = 4 * 1024;
99102
private static final int VERSION = 1;
100103
private final int fd;
101104
private final FilesFacade filesFacade;
@@ -240,7 +243,7 @@ public void sync() {
240243
public void write(long fsn) {
241244
if (closed) return;
242245
long nextGeneration = generation + 1L;
243-
long recordAddress = mmapAddress + (nextGeneration & 1L) * RECORD_SIZE;
246+
long recordAddress = mmapAddress + (nextGeneration & 1L) * RECORD_SLOT_SIZE;
244247
Unsafe.getUnsafe().setMemory(recordAddress, RECORD_SIZE, (byte) 0);
245248
Unsafe.getUnsafe().putInt(recordAddress + MAGIC_OFFSET, FILE_MAGIC);
246249
Unsafe.getUnsafe().putInt(recordAddress + 4, VERSION);
@@ -286,7 +289,7 @@ private static Record readRecord(long address) {
286289

287290
private static Record selectRecord(long address) {
288291
Record first = readRecord(address);
289-
Record second = readRecord(address + RECORD_SIZE);
292+
Record second = readRecord(address + RECORD_SLOT_SIZE);
290293
if (first == null) {
291294
return second;
292295
}

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

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,15 @@
5656
* </ol>
5757
* <p>
5858
* On terminal failure (auth-rejection on reconnect, a cluster-wide durable-ack
59-
* capability gap that exhausts its settle budget, recovery error), the drainer
60-
* drops a {@link OrphanScanner#FAILED_SENTINEL_NAME} sentinel into the slot
61-
* before exiting. Future scans skip the slot until an operator clears the
62-
* sentinel — bounded automatic retry, then human-in-the-loop. A transient
63-
* all-replica failover window is NOT terminal: it is retried indefinitely
64-
* (Invariant B), never quarantined on a wall-clock budget or attempt cap.
59+
* capability gap that exhausts its settle budget, corrupt or incomplete durable
60+
* recovery state), the drainer drops a
61+
* {@link OrphanScanner#FAILED_SENTINEL_NAME} sentinel into the slot before
62+
* exiting. Future scans skip the slot until an operator clears the sentinel —
63+
* bounded automatic retry, then human-in-the-loop. Operational setup failures
64+
* leave no sentinel so a later orphan scan can retry. JVM/programming Errors
65+
* also leave no sentinel and propagate after teardown. A transient all-replica
66+
* failover window is NOT terminal: it is retried indefinitely (Invariant B),
67+
* never quarantined on a wall-clock budget or attempt cap.
6568
*/
6669
public final class BackgroundDrainer implements Runnable {
6770

@@ -406,9 +409,9 @@ public WebSocketClient connectWithDurableAckRetry() {
406409
// retrying cannot clear it, and spinning here would pin
407410
// the slot .lock forever with no .failed sentinel and only
408411
// a throttled, possibly-null-message WARN as a trace.
409-
// Rethrow: run()'s outer catch quarantines the slot
410-
// (markFailed + FAILED) and its finally releases the lock
411-
// -- quarantine-and-exit, exactly as genuine terminals do.
412+
// Rethrow: run() records the failure without attempting
413+
// allocation-heavy logging or a .failed write, its finally
414+
// releases the lock, and then the Error remains visible.
412415
throw (Error) t;
413416
}
414417
// INVARIANT B: a transport failure -- the whole cluster is
@@ -560,15 +563,31 @@ public void run() {
560563
engine = new CursorSendEngine(slotPath, segmentSizeBytes,
561564
sfMaxTotalBytes, CursorSendEngine.DEFAULT_APPEND_DEADLINE_NANOS,
562565
syncIntervalNanos);
563-
} catch (IllegalStateException t) {
564-
String msg = t.getMessage();
565-
if (msg != null && msg.contains("already in use")) {
566-
LOG.info("orphan slot already locked, skipping: {} ({})",
567-
slotPath, msg);
568-
outcome = DrainOutcome.LOCKED_BY_OTHER;
569-
return;
570-
}
566+
} catch (SlotLockContentionException t) {
567+
LOG.info("orphan slot already locked, skipping: {} ({})",
568+
slotPath, t.getMessage());
569+
outcome = DrainOutcome.LOCKED_BY_OTHER;
570+
return;
571+
} catch (SfRecoveryException | MmapSegmentCorruptionException t) {
572+
// The durable chain itself is proven corrupt or incomplete.
573+
// Repeated scans cannot repair it, so preserve the terminal
574+
// quarantine path in the outer catch below.
571575
throw t;
576+
} catch (Exception t) {
577+
// Every other pre-publication construction exception is
578+
// retryable: setup I/O, resource pressure, unexpected setup
579+
// faults, and future operational failures do not prove durable
580+
// data corruption. The constructor has closed partial
581+
// resources; SlotLock retains and retries any unconfirmed
582+
// unlock. Leave no .failed sentinel for the next orphan scan.
583+
// Error deliberately escapes to the outer Error path: it is
584+
// observable after teardown but cannot quarantine intact data.
585+
String msg = t.getMessage();
586+
LOG.warn("drainer setup temporarily unavailable for slot {}: {}",
587+
slotPath, msg);
588+
lastErrorMessage = msg;
589+
outcome = DrainOutcome.FAILED;
590+
return;
572591
}
573592
long target = engine.publishedFsn();
574593
if (engine.ackedFsn() >= target) {
@@ -618,6 +637,16 @@ public void run() {
618637
try {
619638
loop.checkError();
620639
} catch (Throwable t) {
640+
// The I/O loop latches a JVM/programming Error inside a
641+
// LineSenderException so checkError() can cross the
642+
// thread boundary. Preserve the original Error contract:
643+
// no wire quarantine, tear down, then propagate it.
644+
if (t instanceof Error) {
645+
throw (Error) t;
646+
}
647+
if (t.getCause() instanceof Error) {
648+
throw (Error) t.getCause();
649+
}
621650
if (loop.capabilityGapTerminal() != null) {
622651
// Capability gap mid-drain: recycle the wire, NOT
623652
// the slot. connectWithDurableAckRetry() owns the
@@ -673,6 +702,18 @@ public void run() {
673702
// outer condition, which is false for the same reason.
674703
}
675704
outcome = DrainOutcome.STOPPED;
705+
} catch (Error t) {
706+
// Resource pressure and JVM/programming failures prove neither
707+
// durable corruption nor a terminal server response. Log the Error
708+
// best-effort, but never let a secondary logging failure (especially
709+
// under OOME) mask the original Error or prevent teardown.
710+
try {
711+
LOG.error("drainer failed with Error for slot {}", slotPath, t);
712+
} catch (Throwable ignored) {
713+
}
714+
lastErrorMessage = t.getMessage();
715+
outcome = DrainOutcome.FAILED;
716+
throw t;
676717
} catch (Throwable t) {
677718
String msg = t.getMessage();
678719
if (slotPath != null) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
383383
// the file cannot be opened or mapped.
384384
watermarkInProgress = AckWatermark.open(filesFacade, sfDir);
385385
if (watermarkInProgress == null) {
386-
throw new IllegalStateException(
386+
throw new SfOperationalException(
387387
"could not open required ack watermark for SF slot " + sfDir);
388388
}
389389
long baseSeed = lowestBase - 1;
@@ -437,7 +437,7 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man
437437
AckWatermark.removeOrphan(filesFacade, sfDir);
438438
watermarkInProgress = AckWatermark.open(filesFacade, sfDir);
439439
if (watermarkInProgress == null) {
440-
throw new IllegalStateException(
440+
throw new SfOperationalException(
441441
"could not open required ack watermark for SF slot " + sfDir);
442442
}
443443
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@
3030
* the disk is full (the latter surfaces as backpressure on the producer
3131
* via {@link io.questdb.client.cutlass.line.LineSenderException}).
3232
* <p>
33-
* Recovery distinguishes two flavors: this base type marks <b>operational</b>
33+
* Recovery distinguishes three flavors: this base type marks <b>operational</b>
3434
* failures (open/read/mmap/enumeration errors — the file's contents may be fine,
35-
* so recovery must fail closed), while the {@link MmapSegmentCorruptionException}
36-
* subtype marks <b>positively-identified corruption</b> in the file's own
37-
* bytes, which recovery may quarantine instead of aborting.
35+
* so recovery must fail closed); {@link MmapSegmentCorruptionException} marks
36+
* <b>positively-identified corruption</b> in one file's own bytes, which
37+
* recovery may quarantine when the surviving chain proves safe; and
38+
* {@link SfRecoveryException} marks a <b>terminal chain failure</b> that needs
39+
* operator intervention.
3840
*/
3941
public class MmapSegmentException extends RuntimeException {
4042
public MmapSegmentException(String message) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ private long scanMaxGeneration(String dir) {
763763
if (!filesFacade.exists(dir)) return max;
764764
long find = filesFacade.findFirst(dir);
765765
if (find < 0) {
766-
throw new IllegalStateException("could not enumerate SF segment directory " + dir);
766+
throw new SfOperationalException("could not enumerate SF segment directory " + dir);
767767
}
768768
if (find == 0) return max;
769769
try {
@@ -784,7 +784,7 @@ private long scanMaxGeneration(String dir) {
784784
}
785785
}
786786
if (rc < 0) {
787-
throw new IllegalStateException("could not fully enumerate SF segment directory " + dir);
787+
throw new SfOperationalException("could not fully enumerate SF segment directory " + dir);
788788
}
789789
} finally {
790790
filesFacade.findClose(find);

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ static Recovery recover(
273273
// now unreadable) -- fail without mutating. Without one,
274274
// legacy semantics apply: quarantine and start fresh.
275275
if (manifest != null) {
276-
throw new MmapSegmentException("every SF segment in " + sfDir
276+
throw new SfRecoveryException("every SF segment in " + sfDir
277277
+ " is corrupt but " + SfManifest.FILE_NAME
278278
+ " references durable data");
279279
}
@@ -294,7 +294,7 @@ static Recovery recover(
294294
long manifestHeadBase = manifest.headBase();
295295
long manifestActiveBase = manifest.activeBase();
296296
if (manifestHeadBase != manifestActiveBase) {
297-
throw new MmapSegmentException(SfManifest.FILE_NAME + " in " + sfDir
297+
throw new SfRecoveryException(SfManifest.FILE_NAME + " in " + sfDir
298298
+ " references durable data (headBase=" + manifestHeadBase
299299
+ ", activeBase=" + manifestActiveBase
300300
+ ") but no segment files exist");
@@ -328,7 +328,7 @@ static Recovery recover(
328328
}
329329
sortByBaseSeq(data, 0, data.size());
330330
if (manifest == null && requiresManifest) {
331-
throw new MmapSegmentException("new-format SF segment exists but "
331+
throw new SfRecoveryException("new-format SF segment exists but "
332332
+ SfManifest.FILE_NAME + " is missing");
333333
}
334334

@@ -384,19 +384,19 @@ static Recovery recover(
384384
long end = segment.baseSeq() + segment.frameCount();
385385
if (segment.baseSeq() < headBase) {
386386
if (end > headBase) {
387-
throw new MmapSegmentException("segment overlaps committed SF head boundary");
387+
throw new SfRecoveryException("segment overlaps committed SF head boundary");
388388
}
389389
continue; // acknowledged stale file after manifest-before-unlink crash
390390
}
391391
if (segment.baseSeq() > activeBase) {
392-
throw new MmapSegmentException("segment exists beyond committed SF active boundary");
392+
throw new SfRecoveryException("segment exists beyond committed SF active boundary");
393393
}
394394
chain.add(segment);
395395
}
396396
if (chain.size() > 0) {
397397
validateContiguous(chain);
398398
if (chain.get(0).baseSeq() != headBase) {
399-
throw new MmapSegmentException("missing expected SF head segment at base " + headBase);
399+
throw new SfRecoveryException("missing expected SF head segment at base " + headBase);
400400
}
401401
}
402402
active = findActive(all, activeBase);
@@ -438,7 +438,7 @@ static Recovery recover(
438438
}
439439
return Recovery.empty();
440440
}
441-
throw new MmapSegmentException("missing expected SF active segment at base " + activeBase);
441+
throw new SfRecoveryException("missing expected SF active segment at base " + activeBase);
442442
}
443443
if (chain.size() == 0) {
444444
if (headBase != activeBase || active.frameCount() != 0 || corruptPaths != null) {
@@ -448,7 +448,7 @@ static Recovery recover(
448448
// the same provisional baseSeq as a corrupted real
449449
// active -- accepting it would quarantine unacked
450450
// frames and re-issue their FSNs. Fail closed.
451-
throw new MmapSegmentException(
451+
throw new SfRecoveryException(
452452
"missing SF chain between committed boundaries"
453453
+ (corruptPaths != null
454454
? " (a corrupt segment prevents proving the empty state)" : ""));
@@ -467,7 +467,7 @@ static Recovery recover(
467467
// empty-chain acceptance above).
468468
chain.add(active);
469469
} else {
470-
throw new MmapSegmentException(
470+
throw new SfRecoveryException(
471471
"missing expected SF active/tail segment at base " + activeBase);
472472
}
473473
}
@@ -483,7 +483,7 @@ static Recovery recover(
483483
// independently catches sealed segments that LOST frames.
484484
for (int i = 0, n = chain.size() - 1; i < n; i++) {
485485
if (chain.get(i).tornTailBytes() > 0) {
486-
throw new MmapSegmentException("corrupt torn tail in sealed SF segment " + chain.get(i).path());
486+
throw new SfRecoveryException("corrupt torn tail in sealed SF segment " + chain.get(i).path());
487487
}
488488
}
489489
for (int i = 0, n = chain.size(); i < n; i++) {
@@ -738,7 +738,7 @@ private static void validateContiguous(ObjList<MmapSegment> segments) {
738738
MmapSegment current = segments.get(i);
739739
long expected = previous.baseSeq() + previous.frameCount();
740740
if (current.baseSeq() != expected) {
741-
throw new MmapSegmentException("FSN gap in recovered segments: expected "
741+
throw new SfRecoveryException("FSN gap in recovered segments: expected "
742742
+ expected + " but got " + current.baseSeq());
743743
}
744744
}

0 commit comments

Comments
 (0)