Skip to content

Commit 1245c17

Browse files
committed
Make query egress symmetric with sender ingest
Reshape the QuestDB facade so reads and writes share one pooled-lease model, and remove the thread-affine footguns on both sides. Ingest: - Remove QuestDB.sender() and releaseSender(), along with the entire thread-pin subsystem behind them (SenderPool.pinToCurrentThread, releaseCurrentThread, clearPinIfCurrent, the threadAffine ThreadLocal, and the PooledSender invalidated flag that existed only to make pinning safe). borrowSender() is now the only way to lease a Sender. Egress: - Add QuestDB.borrowQuery(), a closeable, non-allocating Query lease that mirrors borrowSender(). Each pooled QueryWorker owns one pre-allocated QueryImpl, handed out reset on borrow; submit() dispatches on the held worker (single-flight) and close() returns it to the pool. The worker no longer auto-releases per query. - Remove query(), newQuery(), and executeSql(). Reads now connect at borrow time rather than submit time; under lazy_connect the read pool still defaults to min=0, so build() does not fail-fast while the server is down. Test seams: - Make the white-box seam constructors public and annotate @testonly where production never calls them (QuestDBImpl, SenderPool). The QueryClientPool connectHook ctor stays public without @testonly because QuestDBImpl constructs the query pool through it. Tests now call these constructors directly instead of via reflection. Update the client tests, the usage example, and the startup/failover design doc to the new API.
1 parent 914c9b5 commit 1245c17

20 files changed

Lines changed: 334 additions & 698 deletions

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,27 @@
2727
import io.questdb.client.cutlass.qwp.client.QwpBindSetter;
2828
import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler;
2929

30+
import java.io.Closeable;
31+
3032
/**
31-
* Per-thread, reusable builder for one query. Obtained from
32-
* {@link QuestDB#query()}: every call on the same thread returns the same
33-
* instance, reset to empty.
33+
* A query handle leased from the {@link QuestDB} pool via
34+
* {@link QuestDB#borrowQuery()}. The handle holds one pooled query client (one
35+
* WebSocket + I/O thread) for the lifetime of the borrow; the caller MUST
36+
* {@link #close()} it to release the client back to the pool (typically via
37+
* try-with-resources).
38+
* <p>
39+
* Allocation: zero at steady state -- {@code borrowQuery()} returns a
40+
* pre-allocated handle bound to the leased pool slot, and the
41+
* {@link Completion} is a field on it reused across submits.
3442
* <p>
3543
* Lifecycle: configure with {@link #sql}, optional {@link #binds}, and
36-
* {@link #handler}, then call {@link #submit()} to obtain a {@link Completion}.
37-
* After the Completion terminates, the next {@code QuestDB.query()} call on
38-
* the same thread returns this same instance with its state reset.
44+
* {@link #handler}, then call {@link #submit()} to obtain a {@link Completion}
45+
* and {@code await()} it before the next {@link #submit()}.
3946
* <p>
40-
* Thread safety: not thread-safe. One in-flight query per thread.
47+
* Thread safety: not thread-safe and single-flight -- one in-flight query per
48+
* handle. To run queries concurrently, borrow one handle per concurrent query.
4149
*/
42-
public interface Query {
50+
public interface Query extends Closeable {
4351

4452
/** Discards the current configuration without submitting. */
4553
void abandon();
@@ -52,6 +60,16 @@ public interface Query {
5260
*/
5361
Query binds(QwpBindSetter binds);
5462

63+
/**
64+
* Releases the leased pooled query client back to the pool. The caller
65+
* MUST call this (typically via try-with-resources). If a submit is still
66+
* in flight, {@code close()} cancels it and waits for the terminal event
67+
* before returning the client. A real disconnect only happens at
68+
* {@link QuestDB#close()}. Idempotent.
69+
*/
70+
@Override
71+
void close();
72+
5573
/**
5674
* Sets the result-batch handler. The handler is invoked on the pooled
5775
* query client's I/O thread; if it touches caller state, it is
@@ -65,11 +83,12 @@ public interface Query {
6583
Query sql(CharSequence sql);
6684

6785
/**
68-
* Submits the query for execution. Returns the {@link Completion} field
69-
* cached on this instance; never allocates. Blocks up to the builder's
70-
* configured acquire timeout if the query pool is exhausted.
86+
* Submits the query for execution on the leased client. Returns the
87+
* {@link Completion} field cached on this handle; never allocates. The
88+
* handle is single-flight: {@code await()} the returned Completion before
89+
* the next {@code submit()}.
7190
*
72-
* @return the single-flight Completion bound to this Query instance
91+
* @return the single-flight Completion bound to this Query handle
7392
*/
7493
Completion submit();
7594
}

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

Lines changed: 29 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424

2525
package io.questdb.client;
2626

27-
import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler;
28-
2927
import java.io.Closeable;
3028

3129
/**
@@ -34,9 +32,9 @@
3432
* share across threads.
3533
* <p>
3634
* Steady-state allocation is zero: pooled instances are pre-allocated and
37-
* reused, the per-thread {@link Query} handle is cached in a {@code ThreadLocal},
38-
* and the {@link Completion} associated with each query is a field on that
39-
* cached handle.
35+
* reused, each borrowed {@link Query} handle is a pre-allocated front bound to
36+
* its pool slot, and the {@link Completion} associated with each query is a
37+
* field on that handle.
4038
* <p>
4139
* Configuration: one {@code ws}/{@code wss} string describes the whole cluster
4240
* (a single {@code addr} server list) and both the ingest and query pools
@@ -47,7 +45,7 @@
4745
* is up).
4846
* <p>
4947
* Thread safety: instances are safe to share. {@link #borrowSender()} and
50-
* {@link #query()} may be called concurrently from any thread; the pool
48+
* {@link #borrowQuery()} may be called concurrently from any thread; the pool
5149
* guarantees mutual exclusion of pooled resources.
5250
*/
5351
public interface QuestDB extends Closeable {
@@ -80,6 +78,31 @@ static QuestDB connect(CharSequence configurationString) {
8078
return builder().fromConfig(configurationString).build();
8179
}
8280

81+
/**
82+
* Borrows a {@link Query} handle from the pool. The caller MUST call
83+
* {@link Query#close()} on the returned instance to release it back to the
84+
* pool (typically via try-with-resources). The handle leases one pooled
85+
* query client (one WebSocket + I/O thread) for the borrow's lifetime;
86+
* submit one or more queries on it, then close it.
87+
* <p>
88+
* Allocation: zero at steady state -- the returned instance is a
89+
* pre-allocated handle bound to the leased pool slot.
90+
* <p>
91+
* Blocking: blocks up to the builder's
92+
* {@link QuestDBBuilder#acquireTimeoutMillis(long) acquire timeout} when
93+
* the pool is exhausted; throws on timeout.
94+
* <p>
95+
* Concurrency: a single handle is single-flight. To run queries
96+
* concurrently, borrow one handle per concurrent query (up to
97+
* {@code query_pool_max}).
98+
*
99+
* @return a Query handle leased from the pool; release with
100+
* {@link Query#close()}
101+
* @throws QueryException if the pool is exhausted beyond the acquire
102+
* timeout, or if this handle is closed
103+
*/
104+
Query borrowQuery();
105+
83106
/**
84107
* Borrows a {@link Sender} from the pool. The caller MUST call
85108
* {@link Sender#close()} on the returned instance to release it back to
@@ -114,61 +137,4 @@ static QuestDB connect(CharSequence configurationString) {
114137
*/
115138
@Override
116139
void close();
117-
118-
/**
119-
* One-shot convenience for queries with no bind parameters. Equivalent to
120-
* {@code query().sql(sql).handler(handler).submit()}. Returns the same
121-
* thread-local {@link Completion} instance that {@link #query()} would,
122-
* so this method is also zero-allocation at steady state.
123-
*
124-
* @param sql the SQL text; the buffer is not retained after submit
125-
* @param handler the result-batch handler; invoked on the pooled query
126-
* client's I/O thread
127-
* @return a single-flight handle for the in-flight query
128-
*/
129-
Completion executeSql(CharSequence sql, QwpColumnBatchHandler handler);
130-
131-
/**
132-
* Allocates a fresh {@link Query} handle. Unlike {@link #query()}, this
133-
* does NOT return the per-thread cached instance; every call allocates.
134-
* <p>
135-
* Use this when one thread needs to hold multiple in-flight queries
136-
* concurrently (each {@code submit()} acquires its own worker from the
137-
* query pool, so up to {@code queryPoolSize} concurrent queries on a
138-
* single thread is fine). For the common case of one query at a time,
139-
* prefer {@link #query()} -- it is allocation-free.
140-
*/
141-
Query newQuery();
142-
143-
/**
144-
* Opens a query builder for the calling thread. Returns the same
145-
* thread-local instance on every call: callers do not need to cache it
146-
* themselves. The returned {@code Query} is in a reset state and is not
147-
* thread-safe -- one in-flight query per thread.
148-
* <p>
149-
* For multiple concurrent in-flight queries from a single thread, use
150-
* {@link #newQuery()} instead.
151-
*/
152-
Query query();
153-
154-
/**
155-
* Releases the thread-affine {@link Sender} (if any) currently attached
156-
* to the calling thread back to the pool. Call this on threads borrowed
157-
* from pools you do not own (for example, Netty event loops) before they
158-
* are recycled, to avoid pinning a {@link Sender} for the lifetime of
159-
* a thread that no longer needs it.
160-
*/
161-
void releaseSender();
162-
163-
/**
164-
* Returns a {@link Sender} pinned to the calling thread. First call on
165-
* a thread takes one from the pool and pins it; subsequent calls on the
166-
* same thread return the same instance. The pin is released by
167-
* {@link #releaseSender()} or by {@link #close()} on this handle.
168-
* <p>
169-
* Use this for long-lived, dedicated producer threads where borrow/return
170-
* overhead would dominate. For short-lived or event-loop callers, prefer
171-
* {@link #borrowSender()}.
172-
*/
173-
Sender sender();
174140
}

core/src/main/java/io/questdb/client/impl/PooledSender.java

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public final class PooledSender implements Sender {
5959
private final int slotIndex;
6060
private volatile long idleSinceMillis;
6161
private volatile boolean inUse;
62-
private volatile boolean invalidated;
6362

6463
PooledSender(Sender delegate, SenderPool pool, int slotIndex) {
6564
this.delegate = delegate;
@@ -141,15 +140,6 @@ public Sender charColumn(CharSequence name, char value) {
141140
* the owning {@code QuestDB} is closed.
142141
* <p>
143142
* Idempotent: a second call after a return is a no-op.
144-
* <p>
145-
* Clears the current thread's pin (if any) before the slot becomes
146-
* borrowable again. Without this step a thread that pinned this
147-
* wrapper and then closed it via the public {@link Sender#close()}
148-
* (the natural try-with-resources idiom) would still hold the pin
149-
* in its {@link ThreadLocal}; a subsequent {@code QuestDB.sender()}
150-
* call on that thread would return the cached wrapper even though
151-
* another consumer has since borrowed the slot, and the two
152-
* consumers would write to the same underlying delegate.
153143
*/
154144
@Override
155145
public void close() {
@@ -167,12 +157,6 @@ public void close() {
167157
flushed = true;
168158
} finally {
169159
inUse = false;
170-
// Clear the pin BEFORE returning the slot. If we cleared
171-
// after giveBack(), a concurrent borrower could grab the
172-
// slot while this thread's pin still references it, and a
173-
// re-pin on this thread would return the (now in-use)
174-
// wrapper -- the same race this clear is meant to close.
175-
pool.clearPinIfCurrent(this);
176160
if (flushed) {
177161
pool.giveBack(this);
178162
} else {
@@ -401,19 +385,11 @@ boolean isInUse() {
401385
return inUse;
402386
}
403387

404-
boolean isInvalidated() {
405-
return invalidated;
406-
}
407-
408388
void markIdleAt(long nowMillis) {
409389
idleSinceMillis = nowMillis;
410390
}
411391

412392
void markInUse() {
413393
inUse = true;
414394
}
415-
416-
void markInvalidated() {
417-
invalidated = true;
418-
}
419395
}

core/src/main/java/io/questdb/client/impl/QueryClientPool.java

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import io.questdb.client.QueryException;
2828
import io.questdb.client.cutlass.qwp.client.QwpQueryClient;
29+
import org.jetbrains.annotations.TestOnly;
2930

3031
import java.util.ArrayDeque;
3132
import java.util.ArrayList;
@@ -89,11 +90,12 @@ public QueryClientPool(
8990
idleTimeoutMillis, maxLifetimeMillis, null);
9091
}
9192

92-
// Package-private constructor exposing the connectHook test seam: production
93-
// passes null (-> the real QwpQueryClient.connect()). White-box tests in
94-
// io.questdb.client.test.impl reach this by reflection to inject a hook that
95-
// throws a non-RuntimeException Throwable from the native connect path.
96-
QueryClientPool(
93+
// Constructor exposing the connectHook seam. Production (QuestDBImpl) passes
94+
// null -> the real QwpQueryClient.connect(); white-box tests pass a hook that
95+
// throws a non-RuntimeException Throwable from the native connect path. This
96+
// is the construction path QuestDBImpl uses, so it is a real (public) ctor,
97+
// not test-only.
98+
public QueryClientPool(
9799
String configurationString,
98100
int minSize,
99101
int maxSize,
@@ -106,13 +108,12 @@ public QueryClientPool(
106108
idleTimeoutMillis, maxLifetimeMillis, connectHook, null);
107109
}
108110

109-
// Package-private constructor exposing both the connectHook and startHook
110-
// test seams: production passes null for each (-> the real
111-
// QwpQueryClient.connect() and QueryWorker.start()). White-box tests in
112-
// io.questdb.client.test.impl reach this by reflection to inject a hook that
113-
// throws a Throwable from either the native connect path (connectHook) or
114-
// the worker thread-start path (startHook).
115-
QueryClientPool(
111+
// Constructor exposing both the connectHook and startHook seams. Production
112+
// reaches it via the overload above (both null -> the real
113+
// QwpQueryClient.connect() and QueryWorker.start()); white-box tests pass a
114+
// hook that throws a Throwable from either the native connect path
115+
// (connectHook) or the worker thread-start path (startHook).
116+
public QueryClientPool(
116117
String configurationString,
117118
int minSize,
118119
int maxSize,
@@ -355,11 +356,12 @@ void release(QueryWorker w) {
355356
}
356357
}
357358

358-
// Package-private white-box accessor for tests: reports the current
359-
// in-flight creation count under the pool lock. A non-zero value after a
360-
// failed acquire() means the slot reservation was never released -- the
361-
// capacity-shrink bug this guards against.
362-
int inFlightCreations() {
359+
// White-box accessor for tests: reports the current in-flight creation count
360+
// under the pool lock. A non-zero value after a failed acquire() means the
361+
// slot reservation was never released -- the capacity-shrink bug this guards
362+
// against.
363+
@TestOnly
364+
public int inFlightCreations() {
363365
lock.lock();
364366
try {
365367
return inFlightCreations;

0 commit comments

Comments
 (0)