Skip to content

Commit 1e8db76

Browse files
committed
address review comments
1 parent 06b6ef7 commit 1e8db76

4 files changed

Lines changed: 166 additions & 31 deletions

File tree

core/src/main/java/io/questdb/client/Sender.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ public int getTimeout() {
614614
private PrivateKey privateKey;
615615
private int protocol = PARAMETER_NOT_SET_EXPLICITLY;
616616
private int protocolVersion = PARAMETER_NOT_SET_EXPLICITLY;
617+
private boolean requestDurableAck;
617618
private int retryTimeoutMillis = PARAMETER_NOT_SET_EXPLICITLY;
618619
private boolean shouldDestroyPrivKey;
619620
private boolean tlsEnabled;
@@ -927,7 +928,8 @@ public Sender build() {
927928
actualAutoFlushIntervalNanos,
928929
actualInFlightWindowSize,
929930
wsAuthHeader,
930-
actualMaxSchemasPerConnection
931+
actualMaxSchemasPerConnection,
932+
requestDurableAck
931933
);
932934
}
933935

@@ -1478,6 +1480,27 @@ public LineSenderBuilder protocolVersion(int protocolVersion) {
14781480
return this;
14791481
}
14801482

1483+
/**
1484+
* Opts the connection in for STATUS_DURABLE_ACK frames. When enabled,
1485+
* servers with primary replication will emit per-table durable-upload
1486+
* watermarks as WAL data reaches the object store.
1487+
* <p>
1488+
* This setting is only supported for WebSocket transport.
1489+
* <p>
1490+
* Observe durable progress via
1491+
* {@link QwpWebSocketSender#getHighestDurableSeqTxn(CharSequence)}.
1492+
*
1493+
* @param enabled true to request durable ACKs
1494+
* @return this instance for method chaining
1495+
*/
1496+
public LineSenderBuilder requestDurableAck(boolean enabled) {
1497+
if (protocol != PARAMETER_NOT_SET_EXPLICITLY && protocol != PROTOCOL_WEBSOCKET) {
1498+
throw new LineSenderException("request_durable_ack is only supported for WebSocket transport");
1499+
}
1500+
this.requestDurableAck = enabled;
1501+
return this;
1502+
}
1503+
14811504
/**
14821505
* Configures the maximum time the Sender will spend retrying upon receiving a recoverable error from the server.
14831506
* <br>
@@ -1870,6 +1893,18 @@ private LineSenderBuilder fromConfig(CharSequence configurationString) {
18701893
pos = getValue(configurationString, pos, sink, "in_flight_window");
18711894
int windowSize = parseIntValue(sink, "in_flight_window");
18721895
inFlightWindowSize(windowSize);
1896+
} else if (Chars.equals("request_durable_ack", sink)) {
1897+
if (protocol != PROTOCOL_WEBSOCKET) {
1898+
throw new LineSenderException("request_durable_ack is only supported for WebSocket transport");
1899+
}
1900+
pos = getValue(configurationString, pos, sink, "request_durable_ack");
1901+
if (Chars.equalsIgnoreCase("on", sink)) {
1902+
requestDurableAck(true);
1903+
} else if (Chars.equalsIgnoreCase("off", sink)) {
1904+
requestDurableAck(false);
1905+
} else {
1906+
throw new LineSenderException("invalid request_durable_ack [value=").put(sink).put(", allowed-values=[on, off]]");
1907+
}
18731908
} else if (Chars.equals("max_schemas_per_connection", sink)) {
18741909
if (protocol != PROTOCOL_WEBSOCKET) {
18751910
throw new LineSenderException("max_schemas_per_connection is only supported for WebSocket transport");

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,21 +1022,25 @@ public QwpWebSocketSender longColumn(CharSequence columnName, long value) {
10221022
}
10231023

10241024
/**
1025-
* Sends a WebSocket PING and reads frames until the PONG comes back,
1026-
* processing any STATUS_DURABLE_ACK or STATUS_OK frames along the way.
1027-
* After this method returns, {@link #getHighestDurableSeqTxn(CharSequence)} reflects
1028-
* the latest durable watermark reported by the server.
1025+
* Sends a WebSocket PING and blocks until the PONG arrives, processing
1026+
* any STATUS_DURABLE_ACK or STATUS_OK frames along the way.
10291027
* <p>
1030-
* In async mode the PING is queued for the I/O thread; this method
1031-
* returns once the I/O thread has sent it and processed the response.
1028+
* The server flushes pending durable ACKs before sending the PONG, so
1029+
* after this method returns, {@link #getHighestDurableSeqTxn(CharSequence)}
1030+
* reflects all durable progress up to the moment the server processed
1031+
* the PING.
1032+
* <p>
1033+
* In async mode the PING is sent by the I/O thread; the I/O loop
1034+
* continues its normal work (sending batches, draining ACKs) while
1035+
* waiting for the PONG.
10321036
*
10331037
* @throws LineSenderException if the connection is closed or the ping times out
10341038
*/
10351039
public void ping() {
10361040
checkNotClosed();
10371041
ensureConnected();
10381042
if (inFlightWindowSize > 1) {
1039-
sendQueue.pingAndDrain();
1043+
sendQueue.ping();
10401044
} else {
10411045
syncPing();
10421046
}

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

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,10 @@ public class WebSocketSendQueue implements QuietCloseable {
110110
// Single pending buffer slot (double-buffering means at most 1 item in queue)
111111
// Zero allocation - just a volatile reference handoff
112112
private volatile MicrobatchBuffer pendingBuffer;
113-
private volatile boolean pingRequested;
114113
private volatile boolean pingComplete;
114+
private volatile boolean pingRequested;
115+
private volatile boolean pongReceived;
116+
private long pingDeadlineNanos;
115117
// Running state
116118
private volatile boolean running;
117119
private volatile boolean shuttingDown;
@@ -347,10 +349,16 @@ public long getDurableSeqTxn(String tableName) {
347349
}
348350

349351
/**
350-
* Requests the I/O thread to send a PING and drain any pending frames
351-
* (including STATUS_DURABLE_ACK). Blocks until the I/O thread completes.
352+
* Requests the I/O thread to send a WebSocket PING and blocks until
353+
* the PONG arrives. The I/O loop continues its normal work (sending
354+
* batches, draining ACKs) while waiting for the PONG.
355+
* <p>
356+
* The server flushes pending durable ACKs before sending the PONG,
357+
* so after this method returns {@code getDurableSeqTxn()} reflects
358+
* all durable progress up to the moment the server processed the PING.
352359
*/
353-
public void pingAndDrain() {
360+
public void ping() {
361+
checkError();
354362
synchronized (processingLock) {
355363
pingComplete = false;
356364
pingRequested = true;
@@ -369,6 +377,7 @@ public void pingAndDrain() {
369377
}
370378
}
371379
}
380+
checkError();
372381
}
373382

374383
/**
@@ -396,12 +405,12 @@ private void checkError() {
396405
}
397406

398407
/**
399-
* Computes the current I/O state based on queue and in-flight status.
408+
* Computes the current I/O state based on queue, in-flight, and ping status.
400409
*/
401410
private IoState computeState(boolean hasInFlight) {
402411
if (!isPendingEmpty()) {
403412
return IoState.ACTIVE;
404-
} else if (hasInFlight) {
413+
} else if (hasInFlight || pingDeadlineNanos > 0) {
405414
return IoState.DRAINING;
406415
} else {
407416
return IoState.IDLE;
@@ -463,18 +472,17 @@ private void ioLoop() {
463472
try {
464473
int drainIdleCycles = 0;
465474
while (running || !isPendingEmpty()) {
475+
// Send a pending PING if requested
466476
if (pingRequested) {
467477
pingRequested = false;
478+
pongReceived = false;
479+
pingDeadlineNanos = System.nanoTime() + InFlightWindow.DEFAULT_TIMEOUT_MS * 1_000_000L;
468480
try {
469481
client.sendPing(1000);
470-
tryReceiveAcks();
471482
} catch (Exception e) {
483+
pingDeadlineNanos = 0;
472484
failTransport(new LineSenderException("Ping failed", e));
473-
} finally {
474-
synchronized (processingLock) {
475-
pingComplete = true;
476-
processingLock.notifyAll();
477-
}
485+
completePing();
478486
}
479487
}
480488

@@ -489,7 +497,7 @@ private void ioLoop() {
489497
// Nothing to do - wait for work under lock
490498
synchronized (processingLock) {
491499
// Re-check under lock to avoid missed wakeup
492-
if (isPendingEmpty() && running) {
500+
if (isPendingEmpty() && running && !pingRequested) {
493501
try {
494502
processingLock.wait(100);
495503
} catch (InterruptedException e) {
@@ -502,10 +510,22 @@ private void ioLoop() {
502510
case ACTIVE:
503511
case DRAINING:
504512
// Try to receive any pending ACKs (non-blocking)
505-
if (hasInFlight && client.isConnected()) {
513+
if (client.isConnected()) {
506514
receivedAcks = tryReceiveAcks();
507515
}
508516

517+
// Check if a pending PING has been answered
518+
if (pingDeadlineNanos > 0) {
519+
if (pongReceived) {
520+
pingDeadlineNanos = 0;
521+
completePing();
522+
} else if (System.nanoTime() >= pingDeadlineNanos) {
523+
pingDeadlineNanos = 0;
524+
failTransport(new LineSenderException("Ping timed out waiting for PONG"));
525+
completePing();
526+
}
527+
}
528+
509529
// Try to dequeue and send a batch
510530
boolean hasWindowSpace = (inFlightWindow == null || inFlightWindow.hasWindowSpace());
511531
if (hasWindowSpace) {
@@ -550,6 +570,13 @@ private void ioLoop() {
550570
}
551571
}
552572

573+
private void completePing() {
574+
synchronized (processingLock) {
575+
pingComplete = true;
576+
processingLock.notifyAll();
577+
}
578+
}
579+
553580
private boolean isPendingEmpty() {
554581
return pendingBuffer == null;
555582
}
@@ -749,6 +776,11 @@ public void onClose(int code, String reason) {
749776
LOG.info("WebSocket closed by server [code={}, reason={}]", code, reason);
750777
failTransport(new LineSenderException("WebSocket closed by server [code=" + code + ", reason=" + reason + ']'));
751778
}
779+
780+
@Override
781+
public void onPong(long payloadPtr, int payloadLen) {
782+
pongReceived = true;
783+
}
752784
}
753785

754786
static final class SeqTxn {

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

Lines changed: 73 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -451,24 +451,31 @@ public void testDurableAckInterleavedWithStatusOk() throws Exception {
451451
}
452452

453453
@Test
454-
public void testPingAndDrainCompletesSuccessfully() throws Exception {
454+
public void testPingBlocksUntilPong() throws Exception {
455455
assertMemoryLeak(() -> {
456456
InFlightWindow window = new InFlightWindow(8, 5_000);
457457
WebSocketSendQueue queue = null;
458458
try (FakeWebSocketClient client = new FakeWebSocketClient()) {
459-
AtomicBoolean ackDelivered = new AtomicBoolean(false);
459+
AtomicInteger callCount = new AtomicInteger();
460460
client.setTryReceiveBehavior(handler -> {
461-
if (ackDelivered.compareAndSet(false, true)) {
462-
emitDurableAck(handler, "t", 7);
463-
return true;
461+
int n = callCount.getAndIncrement();
462+
switch (n) {
463+
case 0:
464+
emitDurableAck(handler, "t", 7);
465+
return true;
466+
case 1:
467+
handler.onPong(0, 0);
468+
return true;
469+
default:
470+
return false;
464471
}
465-
return false;
466472
});
467473

468474
queue = new WebSocketSendQueue(client, window, 1_000, 500);
469475

470-
queue.pingAndDrain();
476+
queue.ping();
471477

478+
// After ping() returns, durable ACK must already be processed
472479
assertEquals(7, queue.getDurableSeqTxn("t"));
473480
} finally {
474481
closeQuietly(queue);
@@ -477,7 +484,7 @@ public void testPingAndDrainCompletesSuccessfully() throws Exception {
477484
}
478485

479486
@Test
480-
public void testPingAndDrainWithInFlightBatches() throws Exception {
487+
public void testPingWithInFlightBatches() throws Exception {
481488
assertMemoryLeak(() -> {
482489
InFlightWindow window = new InFlightWindow(8, 5_000);
483490
WebSocketSendQueue queue = null;
@@ -495,14 +502,17 @@ public void testPingAndDrainWithInFlightBatches() throws Exception {
495502
case 1:
496503
emitDurableAck(handler, "t", 5);
497504
return true;
505+
case 2:
506+
handler.onPong(0, 0);
507+
return true;
498508
default:
499509
return false;
500510
}
501511
});
502512

503513
queue = new WebSocketSendQueue(client, window, 1_000, 500);
504514

505-
queue.pingAndDrain();
515+
queue.ping();
506516

507517
assertEquals(0, window.getInFlightCount());
508518
assertEquals(5, queue.getDurableSeqTxn("t"));
@@ -512,6 +522,54 @@ public void testPingAndDrainWithInFlightBatches() throws Exception {
512522
});
513523
}
514524

525+
@Test
526+
public void testPingTimesOutWhenNoPong() throws Exception {
527+
assertMemoryLeak(() -> {
528+
InFlightWindow window = new InFlightWindow(8, 5_000);
529+
WebSocketSendQueue queue = null;
530+
try (FakeWebSocketClient client = new FakeWebSocketClient()) {
531+
// Never emit a PONG
532+
client.setTryReceiveBehavior(handler -> false);
533+
534+
queue = new WebSocketSendQueue(client, window, 1_000, 500);
535+
536+
try {
537+
queue.ping();
538+
fail("Expected ping timeout");
539+
} catch (LineSenderException e) {
540+
assertTrue(e.getMessage().contains("Ping timed out"));
541+
}
542+
} finally {
543+
closeQuietly(queue);
544+
}
545+
});
546+
}
547+
548+
@Test
549+
public void testPingSurfacesTransportError() throws Exception {
550+
assertMemoryLeak(() -> {
551+
InFlightWindow window = new InFlightWindow(8, 5_000);
552+
WebSocketSendQueue queue = null;
553+
try (FakeWebSocketClient client = new FakeWebSocketClient()) {
554+
client.setPingSendBehavior(() -> {
555+
throw new RuntimeException("ping-send-fail");
556+
});
557+
558+
queue = new WebSocketSendQueue(client, window, 1_000, 500);
559+
560+
try {
561+
queue.ping();
562+
fail("Expected error from ping");
563+
} catch (LineSenderException e) {
564+
assertTrue(e.getMessage().contains("Ping failed")
565+
|| e.getMessage().contains("Error in send queue"));
566+
}
567+
} finally {
568+
closeQuietly(queue);
569+
}
570+
});
571+
}
572+
515573
@Test
516574
public void testDurableSeqTxnInitiallyMinusOne() throws Exception {
517575
assertMemoryLeak(() -> {
@@ -603,6 +661,7 @@ private interface ReceiveBehavior {
603661
private static class FakeWebSocketClient extends WebSocketClient {
604662
private volatile TryReceiveBehavior behavior = handler -> false;
605663
private volatile boolean connected = true;
664+
private volatile Runnable pingSendBehavior = () -> {};
606665
private volatile ReceiveBehavior receiveBehavior = (handler, timeout) -> false;
607666
private volatile SendBehavior sendBehavior = (dataPtr, length) -> {
608667
};
@@ -629,6 +688,11 @@ public void sendBinary(long dataPtr, int length) {
629688

630689
@Override
631690
public void sendPing(int timeout) {
691+
pingSendBehavior.run();
692+
}
693+
694+
public void setPingSendBehavior(Runnable pingSendBehavior) {
695+
this.pingSendBehavior = pingSendBehavior;
632696
}
633697

634698
public void setSendBehavior(SendBehavior sendBehavior) {

0 commit comments

Comments
 (0)