From 8acc30066b9e2fc0903244972ff55d682b1bc6e5 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 03:58:03 +0100 Subject: [PATCH 01/12] chore(qwp): new pooling API for the java QWP client --- .../java/io/questdb/client/Completion.java | 79 ++++ .../main/java/io/questdb/client/Query.java | 75 ++++ .../io/questdb/client/QueryException.java | 59 +++ .../main/java/io/questdb/client/QuestDB.java | 186 +++++++++ .../io/questdb/client/QuestDBBuilder.java | 268 +++++++++++++ .../main/java/io/questdb/client/Sender.java | 56 +++ .../client/cutlass/qwp/client/ColumnView.java | 24 ++ .../cutlass/qwp/client/QwpColumnBatch.java | 55 +++ .../qwp/client/QwpColumnBatchHandler.java | 61 +++ .../cutlass/qwp/client/QwpQueryClient.java | 65 ++- .../qwp/client/QwpWebSocketSender.java | 173 ++++---- .../client/cutlass/qwp/client/RowView.java | 24 ++ .../client/impl/ConfigStringTranslator.java | 376 ++++++++++++++++++ .../questdb/client/impl/PoolHousekeeper.java | 92 +++++ .../io/questdb/client/impl/PooledSender.java | 360 +++++++++++++++++ .../questdb/client/impl/QueryClientPool.java | 247 ++++++++++++ .../io/questdb/client/impl/QueryImpl.java | 295 ++++++++++++++ .../io/questdb/client/impl/QueryWorker.java | 168 ++++++++ .../io/questdb/client/impl/QuestDBImpl.java | 129 ++++++ .../io/questdb/client/impl/SenderPool.java | 301 ++++++++++++++ .../client/test/QuestDBBuilderTest.java | 135 +++++++ .../client/test/example/QuestDBExamples.java | 199 +++++++++ .../test/impl/ConfigStringTranslatorTest.java | 159 ++++++++ .../client/test/impl/QueryWorkerTest.java | 49 +++ .../client/test/impl/SenderPoolTest.java | 258 ++++++++++++ 25 files changed, 3780 insertions(+), 113 deletions(-) create mode 100644 core/src/main/java/io/questdb/client/Completion.java create mode 100644 core/src/main/java/io/questdb/client/Query.java create mode 100644 core/src/main/java/io/questdb/client/QueryException.java create mode 100644 core/src/main/java/io/questdb/client/QuestDB.java create mode 100644 core/src/main/java/io/questdb/client/QuestDBBuilder.java create mode 100644 core/src/main/java/io/questdb/client/impl/ConfigStringTranslator.java create mode 100644 core/src/main/java/io/questdb/client/impl/PoolHousekeeper.java create mode 100644 core/src/main/java/io/questdb/client/impl/PooledSender.java create mode 100644 core/src/main/java/io/questdb/client/impl/QueryClientPool.java create mode 100644 core/src/main/java/io/questdb/client/impl/QueryImpl.java create mode 100644 core/src/main/java/io/questdb/client/impl/QueryWorker.java create mode 100644 core/src/main/java/io/questdb/client/impl/QuestDBImpl.java create mode 100644 core/src/main/java/io/questdb/client/impl/SenderPool.java create mode 100644 core/src/test/java/io/questdb/client/test/QuestDBBuilderTest.java create mode 100644 core/src/test/java/io/questdb/client/test/example/QuestDBExamples.java create mode 100644 core/src/test/java/io/questdb/client/test/impl/ConfigStringTranslatorTest.java create mode 100644 core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java create mode 100644 core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java diff --git a/core/src/main/java/io/questdb/client/Completion.java b/core/src/main/java/io/questdb/client/Completion.java new file mode 100644 index 00000000..0888370d --- /dev/null +++ b/core/src/main/java/io/questdb/client/Completion.java @@ -0,0 +1,79 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client; + +import java.util.concurrent.TimeUnit; + +/** + * Async handle for a submitted {@link Query}. Returned by {@link Query#submit()}. + *

+ * Lifecycle: the Completion is allocated once as a field on the per-thread + * {@link Query} instance and is reused on every {@code submit()}. It is + * single-flight: a new {@code submit()} cannot be issued on the same {@link Query} + * until the previous Completion resolves (via {@link #await()}, + * {@link #await(long, TimeUnit)} returning {@code true}, or an explicit + * {@link #cancel()} that races to terminal). + *

+ * Signaling: the Completion is signaled from the I/O thread of the pooled + * query client when the handler's terminal callback ({@code onEnd}, + * {@code onError}, or {@code onExecDone}) returns. + */ +public interface Completion { + + /** + * Blocks until the query completes. Rethrows any server-reported failure + * as a {@link QueryException}. Returns normally on success. + * + * @throws QueryException if the server reported an error or + * {@link #cancel()} won the race + * @throws InterruptedException if the calling thread is interrupted + * while waiting + */ + void await() throws InterruptedException; + + /** + * Blocks up to the given timeout. Returns {@code true} if the query + * completed, {@code false} on timeout. + * + * @throws QueryException if the server reported an error or + * {@link #cancel()} won the race + * @throws InterruptedException if the calling thread is interrupted + * while waiting + */ + boolean await(long timeout, TimeUnit unit) throws InterruptedException; + + /** + * Requests cancellation of the in-flight query. The handler's + * {@code onError} fires with a cancellation status. No-op if the query + * has already completed. + */ + void cancel(); + + /** + * Returns true once the query has terminated (success, error, or cancel + * acknowledged). + */ + boolean isDone(); +} diff --git a/core/src/main/java/io/questdb/client/Query.java b/core/src/main/java/io/questdb/client/Query.java new file mode 100644 index 00000000..f6832e84 --- /dev/null +++ b/core/src/main/java/io/questdb/client/Query.java @@ -0,0 +1,75 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client; + +import io.questdb.client.cutlass.qwp.client.QwpBindSetter; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler; + +/** + * Per-thread, reusable builder for one query. Obtained from + * {@link QuestDB#query()}: every call on the same thread returns the same + * instance, reset to empty. + *

+ * Lifecycle: configure with {@link #sql}, optional {@link #binds}, and + * {@link #handler}, then call {@link #submit()} to obtain a {@link Completion}. + * After the Completion terminates, the next {@code QuestDB.query()} call on + * the same thread returns this same instance with its state reset. + *

+ * Thread safety: not thread-safe. One in-flight query per thread. + */ +public interface Query { + + /** Discards the current configuration without submitting. */ + void abandon(); + + /** + * Sets the bind-value setter, invoked by the pooled query client when the + * QUERY_REQUEST frame is being prepared. Pass a reusable + * {@link QwpBindSetter} instance (or a stateless lambda hoisted to a + * field) to keep submission zero-allocation. + */ + Query binds(QwpBindSetter binds); + + /** + * Sets the result-batch handler. The handler is invoked on the pooled + * query client's I/O thread; if it touches caller state, it is + * responsible for its own synchronization. + */ + Query handler(QwpColumnBatchHandler handler); + + /** + * Sets the SQL text. The buffer is not retained past {@link #submit()}. + */ + Query sql(CharSequence sql); + + /** + * Submits the query for execution. Returns the {@link Completion} field + * cached on this instance; never allocates. Blocks up to the builder's + * configured acquire timeout if the query pool is exhausted. + * + * @return the single-flight Completion bound to this Query instance + */ + Completion submit(); +} diff --git a/core/src/main/java/io/questdb/client/QueryException.java b/core/src/main/java/io/questdb/client/QueryException.java new file mode 100644 index 00000000..a3fdedbd --- /dev/null +++ b/core/src/main/java/io/questdb/client/QueryException.java @@ -0,0 +1,59 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client; + +/** + * Thrown from {@link Completion#await()} / {@link Completion#await(long, java.util.concurrent.TimeUnit)} + * when the server reported an error for the corresponding {@link Query}, + * when {@link Completion#cancel()} won the race, or when an unrecoverable + * transport failure occurred during submission. + *

+ * The original wire-level status byte is exposed via {@link #getStatus()} so + * callers can distinguish cancellation from schema errors etc. without + * string-matching the message. + */ +public class QueryException extends RuntimeException { + + private final byte status; + + public QueryException(byte status, String message) { + super(message); + this.status = status; + } + + public QueryException(byte status, String message, Throwable cause) { + super(message, cause); + this.status = status; + } + + /** + * Returns the server-reported wire status byte (see QWP protocol + * definitions), or {@code 0} if this exception was raised by the client + * (for example, transport failure before any server response). + */ + public byte getStatus() { + return status; + } +} diff --git a/core/src/main/java/io/questdb/client/QuestDB.java b/core/src/main/java/io/questdb/client/QuestDB.java new file mode 100644 index 00000000..b90e66fd --- /dev/null +++ b/core/src/main/java/io/questdb/client/QuestDB.java @@ -0,0 +1,186 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client; + +import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler; + +import java.io.Closeable; + +/** + * High-level handle to a QuestDB deployment. Owns connection pools for both + * ingest (via {@link Sender}) and egress (via {@link Query}). Construct once, + * share across threads. + *

+ * Steady-state allocation is zero: pooled instances are pre-allocated and + * reused, the per-thread {@link Query} handle is cached in a {@code ThreadLocal}, + * and the {@link Completion} associated with each query is a field on that + * cached handle. + *

+ * Configuration: use {@link #connect(CharSequence)} when the same address list + * and credentials serve both ingest and egress -- the most common case. + * Use {@link #connect(CharSequence, CharSequence)} or {@link #builder()} when + * ingest and egress endpoints differ. + *

+ * Thread safety: instances are safe to share. {@link #borrowSender()} and + * {@link #query()} may be called concurrently from any thread; the pool + * guarantees mutual exclusion of pooled resources. + */ +public interface QuestDB extends Closeable { + + /** + * Builder for advanced configuration (pool sizes, acquisition timeouts, + * differing ingest/egress configs). + */ + static QuestDBBuilder builder() { + return new QuestDBBuilder(); + } + + /** + * Connects with a single configuration string used for both ingest and + * egress. The schema must be {@code http}, {@code https}, {@code ws} or + * {@code wss}; the other half of the deployment is derived by schema + * translation ({@code http}<->{@code ws}, {@code https}<->{@code wss}). + *

+ * Use {@link #connect(CharSequence, CharSequence)} or {@link #builder()} + * for ingest transports other than HTTP/HTTPS, or when ingest and egress + * use different addresses. + * + * @param configurationString a Sender- or QwpQueryClient-style config + * string (see {@link Sender#fromConfig} or + * {@link io.questdb.client.cutlass.qwp.client.QwpQueryClient#fromConfig}) + * @return a connected QuestDB handle + */ + static QuestDB connect(CharSequence configurationString) { + return builder().fromConfig(configurationString).build(); + } + + /** + * Connects with explicit ingest and egress configuration strings. + * + * @param ingestConfigurationString config for the {@link Sender} pool + * ({@link Sender#fromConfig} format) + * @param queryConfigurationString config for the query pool + * ({@link io.questdb.client.cutlass.qwp.client.QwpQueryClient#fromConfig} format) + * @return a connected QuestDB handle + */ + static QuestDB connect(CharSequence ingestConfigurationString, CharSequence queryConfigurationString) { + return builder() + .ingestConfig(ingestConfigurationString) + .queryConfig(queryConfigurationString) + .build(); + } + + /** + * Borrows a {@link Sender} from the pool. The caller MUST call + * {@link Sender#close()} on the returned instance to release it back to + * the pool. {@code close()} on a pooled Sender flushes pending rows + * before returning to the pool; a real disconnect only happens at + * {@link #close()} on this {@code QuestDB} handle. + *

+ * Allocation: zero at steady state -- the returned instance is a + * pre-allocated decorator backed by a pre-allocated underlying Sender. + *

+ * Blocking: blocks up to the builder's + * {@link QuestDBBuilder#acquireTimeoutMillis(long) acquire timeout} when + * the pool is exhausted; throws on timeout. + * + * @return a Sender leased from the pool; release with {@link Sender#close()} + * @throws io.questdb.client.cutlass.line.LineSenderException if the pool + * is exhausted + * beyond the + * acquire + * timeout, or + * if this + * handle is + * closed + */ + Sender borrowSender(); + + /** + * Shuts down the pools, closing every underlying {@link Sender} and + * query client. Idempotent. Threads currently blocked in + * {@link #borrowSender()} or {@link Query#submit()} are released with an + * error. + */ + @Override + void close(); + + /** + * One-shot convenience for queries with no bind parameters. Equivalent to + * {@code query().sql(sql).handler(handler).submit()}. Returns the same + * thread-local {@link Completion} instance that {@link #query()} would, + * so this method is also zero-allocation at steady state. + * + * @param sql the SQL text; the buffer is not retained after submit + * @param handler the result-batch handler; invoked on the pooled query + * client's I/O thread + * @return a single-flight handle for the in-flight query + */ + Completion executeSql(CharSequence sql, QwpColumnBatchHandler handler); + + /** + * Allocates a fresh {@link Query} handle. Unlike {@link #query()}, this + * does NOT return the per-thread cached instance; every call allocates. + *

+ * Use this when one thread needs to hold multiple in-flight queries + * concurrently (each {@code submit()} acquires its own worker from the + * query pool, so up to {@code queryPoolSize} concurrent queries on a + * single thread is fine). For the common case of one query at a time, + * prefer {@link #query()} -- it is allocation-free. + */ + Query newQuery(); + + /** + * Opens a query builder for the calling thread. Returns the same + * thread-local instance on every call: callers do not need to cache it + * themselves. The returned {@code Query} is in a reset state and is not + * thread-safe -- one in-flight query per thread. + *

+ * For multiple concurrent in-flight queries from a single thread, use + * {@link #newQuery()} instead. + */ + Query query(); + + /** + * Releases the thread-affine {@link Sender} (if any) currently attached + * to the calling thread back to the pool. Call this on threads borrowed + * from pools you do not own (for example, Netty event loops) before they + * are recycled, to avoid pinning a {@link Sender} for the lifetime of + * a thread that no longer needs it. + */ + void releaseSender(); + + /** + * Returns a {@link Sender} pinned to the calling thread. First call on + * a thread takes one from the pool and pins it; subsequent calls on the + * same thread return the same instance. The pin is released by + * {@link #releaseSender()} or by {@link #close()} on this handle. + *

+ * Use this for long-lived, dedicated producer threads where borrow/return + * overhead would dominate. For short-lived or event-loop callers, prefer + * {@link #borrowSender()}. + */ + Sender sender(); +} diff --git a/core/src/main/java/io/questdb/client/QuestDBBuilder.java b/core/src/main/java/io/questdb/client/QuestDBBuilder.java new file mode 100644 index 00000000..7a037698 --- /dev/null +++ b/core/src/main/java/io/questdb/client/QuestDBBuilder.java @@ -0,0 +1,268 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client; + +import io.questdb.client.impl.ConfigStringTranslator; +import io.questdb.client.impl.QuestDBImpl; + +/** + * Builder for {@link QuestDB}. Most callers use {@link QuestDB#connect(CharSequence)}; + * this builder is for pool sizing, idle/lifetime knobs, acquire timeout, + * and the case where ingest and egress configs differ. + */ +public final class QuestDBBuilder { + + static final long DEFAULT_ACQUIRE_TIMEOUT_MILLIS = 5_000; + static final long DEFAULT_HOUSEKEEPER_INTERVAL_MILLIS = 5_000; + static final long DEFAULT_IDLE_TIMEOUT_MILLIS = 60_000; + static final long DEFAULT_MAX_LIFETIME_MILLIS = 30 * 60_000L; + static final int DEFAULT_POOL_MAX = 4; + static final int DEFAULT_POOL_MIN = 1; + + private long acquireTimeoutMillis = DEFAULT_ACQUIRE_TIMEOUT_MILLIS; + private long housekeeperIntervalMillis = DEFAULT_HOUSEKEEPER_INTERVAL_MILLIS; + private long idleTimeoutMillis = DEFAULT_IDLE_TIMEOUT_MILLIS; + private String ingestConfig; + private long maxLifetimeMillis = DEFAULT_MAX_LIFETIME_MILLIS; + private String queryConfig; + private int queryPoolMax = DEFAULT_POOL_MAX; + private int queryPoolMin = DEFAULT_POOL_MIN; + private int senderPoolMax = DEFAULT_POOL_MAX; + private int senderPoolMin = DEFAULT_POOL_MIN; + + QuestDBBuilder() { + } + + /** + * Maximum time {@link QuestDB#borrowSender()} and {@link Query#submit()} + * block when the pool is exhausted (every slot in use and {@code max} + * already reached) before throwing. Defaults to 5000ms. + */ + public QuestDBBuilder acquireTimeoutMillis(long millis) { + if (millis < 0) { + throw new IllegalArgumentException("acquireTimeoutMillis must be >= 0"); + } + this.acquireTimeoutMillis = millis; + return this; + } + + /** + * Builds the {@link QuestDB} handle. Eagerly creates {@code min} + * connections in each pool; further slots are allocated lazily up to + * {@code max} when load demands and reaped back to {@code min} when + * idle. + */ + public QuestDB build() { + if (ingestConfig == null) { + throw new IllegalStateException("ingest configuration is required; call fromConfig() or ingestConfig()"); + } + if (queryConfig == null) { + throw new IllegalStateException("query configuration is required; call fromConfig() or queryConfig()"); + } + return new QuestDBImpl( + ingestConfig, + queryConfig, + senderPoolMin, + senderPoolMax, + queryPoolMin, + queryPoolMax, + acquireTimeoutMillis, + idleTimeoutMillis, + maxLifetimeMillis, + housekeeperIntervalMillis + ); + } + + /** + * Sets a single unified configuration string used to derive both the + * ingest and the egress config. Schema must be {@code http}, {@code https}, + * {@code ws} or {@code wss}; the other half is derived by schema + * translation. + */ + public QuestDBBuilder fromConfig(CharSequence configurationString) { + ConfigStringTranslator.Bundle bundle = ConfigStringTranslator.deriveBothSides(configurationString); + this.ingestConfig = bundle.ingestConfig; + this.queryConfig = bundle.queryConfig; + ConfigStringTranslator.PoolConfig pc = bundle.poolConfig; + // Apply pool keys carried in the string. Explicit builder calls AFTER + // fromConfig() will overwrite these -- last write wins. + if (pc.senderPoolMin != ConfigStringTranslator.PoolConfig.UNSET) { + senderPoolMin(pc.senderPoolMin); + } + if (pc.senderPoolMax != ConfigStringTranslator.PoolConfig.UNSET) { + senderPoolMax(pc.senderPoolMax); + } + if (pc.queryPoolMin != ConfigStringTranslator.PoolConfig.UNSET) { + queryPoolMin(pc.queryPoolMin); + } + if (pc.queryPoolMax != ConfigStringTranslator.PoolConfig.UNSET) { + queryPoolMax(pc.queryPoolMax); + } + if (pc.acquireTimeoutMillis != ConfigStringTranslator.PoolConfig.UNSET) { + acquireTimeoutMillis(pc.acquireTimeoutMillis); + } + if (pc.idleTimeoutMillis != ConfigStringTranslator.PoolConfig.UNSET) { + idleTimeoutMillis(pc.idleTimeoutMillis); + } + if (pc.maxLifetimeMillis != ConfigStringTranslator.PoolConfig.UNSET) { + maxLifetimeMillis(pc.maxLifetimeMillis); + } + if (pc.housekeeperIntervalMillis != ConfigStringTranslator.PoolConfig.UNSET) { + housekeeperIntervalMillis(pc.housekeeperIntervalMillis); + } + return this; + } + + /** + * Sweep interval for the daemon housekeeper that reaps idle and over-age + * pool slots. Defaults to 5000ms. Reduce if you set very short + * {@link #idleTimeoutMillis} values; otherwise the default is fine. + */ + public QuestDBBuilder housekeeperIntervalMillis(long millis) { + if (millis < 100) { + throw new IllegalArgumentException("housekeeperIntervalMillis must be >= 100"); + } + this.housekeeperIntervalMillis = millis; + return this; + } + + /** + * How long a connection may remain idle in the pool before the + * housekeeper closes it. {@code minSize} is always respected -- the pool + * never shrinks below it. Defaults to 60000ms. + */ + public QuestDBBuilder idleTimeoutMillis(long millis) { + if (millis < 0) { + throw new IllegalArgumentException("idleTimeoutMillis must be >= 0"); + } + this.idleTimeoutMillis = millis == 0 ? Long.MAX_VALUE : millis; + return this; + } + + /** + * Sets the ingest-side configuration in {@link Sender#fromConfig} format. + */ + public QuestDBBuilder ingestConfig(CharSequence configurationString) { + this.ingestConfig = configurationString.toString(); + return this; + } + + /** + * Maximum age of a pooled connection before the housekeeper recycles it + * (next time it is idle). Useful for picking up DNS / load-balancer + * changes and bounding leaked server state. Defaults to 30 minutes. + */ + public QuestDBBuilder maxLifetimeMillis(long millis) { + if (millis < 0) { + throw new IllegalArgumentException("maxLifetimeMillis must be >= 0"); + } + this.maxLifetimeMillis = millis == 0 ? Long.MAX_VALUE : millis; + return this; + } + + /** + * Sets the query-side configuration in + * {@link io.questdb.client.cutlass.qwp.client.QwpQueryClient#fromConfig} + * format. + */ + public QuestDBBuilder queryConfig(CharSequence configurationString) { + this.queryConfig = configurationString.toString(); + return this; + } + + /** + * Maximum query-pool size. Defaults to 4. + */ + public QuestDBBuilder queryPoolMax(int max) { + if (max < 1) { + throw new IllegalArgumentException("queryPoolMax must be >= 1"); + } + this.queryPoolMax = max; + return this; + } + + /** + * Minimum query-pool size (always kept warm). Defaults to 1. Set to 0 + * to allow the pool to drain fully when idle. + */ + public QuestDBBuilder queryPoolMin(int min) { + if (min < 0) { + throw new IllegalArgumentException("queryPoolMin must be >= 0"); + } + this.queryPoolMin = min; + return this; + } + + /** + * Fixed query-pool size shortcut: equivalent to + * {@code queryPoolMin(size).queryPoolMax(size)}. Eager allocation, + * no growth or reaping -- matches the original (non-elastic) behavior. + */ + public QuestDBBuilder queryPoolSize(int size) { + if (size < 1) { + throw new IllegalArgumentException("queryPoolSize must be >= 1"); + } + this.queryPoolMin = size; + this.queryPoolMax = size; + return this; + } + + /** + * Maximum sender-pool size. Defaults to 4. + */ + public QuestDBBuilder senderPoolMax(int max) { + if (max < 1) { + throw new IllegalArgumentException("senderPoolMax must be >= 1"); + } + this.senderPoolMax = max; + return this; + } + + /** + * Minimum sender-pool size (always kept warm). Defaults to 1. Set to 0 + * to allow the pool to drain fully when idle. + */ + public QuestDBBuilder senderPoolMin(int min) { + if (min < 0) { + throw new IllegalArgumentException("senderPoolMin must be >= 0"); + } + this.senderPoolMin = min; + return this; + } + + /** + * Fixed sender-pool size shortcut: equivalent to + * {@code senderPoolMin(size).senderPoolMax(size)}. Eager allocation, + * no growth or reaping -- matches the original (non-elastic) behavior. + */ + public QuestDBBuilder senderPoolSize(int size) { + if (size < 1) { + throw new IllegalArgumentException("senderPoolSize must be >= 1"); + } + this.senderPoolMin = size; + this.senderPoolMax = size; + return this; + } +} diff --git a/core/src/main/java/io/questdb/client/Sender.java b/core/src/main/java/io/questdb/client/Sender.java index 990afb11..5354a89c 100644 --- a/core/src/main/java/io/questdb/client/Sender.java +++ b/core/src/main/java/io/questdb/client/Sender.java @@ -262,6 +262,27 @@ static Sender fromEnv() { */ void atNow(); + /** + * Block until the server has acknowledged every frame up to {@code targetFsn}, + * or until {@code timeoutMillis} elapses. Pair with {@link #flushAndGetSequence()} + * to obtain {@code targetFsn} for a specific flush. + *
+ * When {@code request_durable_ack=on} (Enterprise primary replication), {@code targetFsn} + * advances after durable upload to object storage, not on the ordinary commit ACK. + *
+ * Only the WebSocket QWP transport tracks frame sequence numbers. On other transports + * (HTTP, TCP, UDP) the call returns immediately: {@code true} when {@code targetFsn < 0} + * (nothing to wait for), {@code false} otherwise. + * + * @param targetFsn FSN to wait for; typically the return value of {@link #flushAndGetSequence()} + * @param timeoutMillis upper bound on the wait; {@code <= 0} returns the current state without blocking + * @return {@code true} if the server has acknowledged up to {@code targetFsn} on return, {@code false} on timeout + * @throws LineSenderException if the transport has latched a terminal error + */ + default boolean awaitAckedFsn(long targetFsn, long timeoutMillis) { + return targetFsn < 0L; + } + /** * Add a BINARY column value as a byte array. The bytes are written verbatim * with no encoding or transformation. To mark the value NULL, do not call @@ -474,6 +495,26 @@ default Sender floatColumn(CharSequence name, float value) { */ void flush(); + /** + * Same as {@link #flush()} but returns the highest frame sequence number (FSN) the + * call published. Producer-side correlation handle: log + * {@code (returnedFsn, domainContext)} alongside the data, then join to the + * {@link SenderError#getFromFsn()} / {@link SenderError#getToFsn()} span when an + * async error is delivered, or pass it to {@link #awaitAckedFsn(long, long)} for + * a bounded blocking wait. + *
+ * Returns {@code -1} when nothing was published by this call, and on transports that + * do not track frame sequence numbers (HTTP, TCP, UDP). + * + * @return highest FSN published by this call, or {@code -1} if no data was published + * or the transport does not expose FSNs + * @throws LineSenderException under the same conditions as {@link #flush()} + */ + default long flushAndGetSequence() { + flush(); + return -1L; + } + /** * Add a GEOHASH column value from pre-packed bits and an explicit bit precision. *

@@ -523,6 +564,21 @@ default Sender geoHashColumn(CharSequence name, CharSequence value) { throw new LineSenderException("current protocol version does not support geohash"); } + /** + * Highest frame sequence number (FSN) the server has acknowledged, or that the sender + * has skipped past on a {@link SenderError.Policy#DROP_AND_CONTINUE} rejection. + * Returns {@code -1} when no batch has been published yet, and on transports that + * do not track FSNs (HTTP, TCP, UDP). + *
+ * Snapshot accessor: for a bounded blocking wait, use + * {@link #awaitAckedFsn(long, long)}. + * + * @return highest acknowledged FSN, or {@code -1} if none or unsupported + */ + default long getAckedFsn() { + return -1L; + } + /** * Add a column with a 32-bit signed integer value. * diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/ColumnView.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/ColumnView.java index ecef52d1..db7a51ba 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/ColumnView.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/ColumnView.java @@ -25,6 +25,9 @@ package io.questdb.client.cutlass.qwp.client; import io.questdb.client.cutlass.qwp.protocol.QwpConstants; +import io.questdb.client.std.Decimal128; +import io.questdb.client.std.Decimal256; +import io.questdb.client.std.Decimal64; import io.questdb.client.std.Long256Sink; import io.questdb.client.std.Unsafe; import io.questdb.client.std.Uuid; @@ -178,6 +181,13 @@ public char getCharValue(int row) { return (char) Unsafe.getUnsafe().getShort(layout.valuesAddr + 2L * layout.denseIndex(row)); } + /** + * @see QwpColumnBatch#getDecimal128(int, int, Decimal128) + */ + public boolean getDecimal128(int row, Decimal128 sink) { + return batch.getDecimal128(col, row, sink); + } + /** * @see QwpColumnBatch#getDecimal128High(int, int) */ @@ -194,6 +204,20 @@ public long getDecimal128Low(int row) { return Unsafe.getUnsafe().getLong(layout.valuesAddr + 16L * layout.denseIndex(row)); } + /** + * @see QwpColumnBatch#getDecimal256(int, int, Decimal256) + */ + public boolean getDecimal256(int row, Decimal256 sink) { + return batch.getDecimal256(col, row, sink); + } + + /** + * @see QwpColumnBatch#getDecimal64(int, int, Decimal64) + */ + public boolean getDecimal64(int row, Decimal64 sink) { + return batch.getDecimal64(col, row, sink); + } + /** * @see QwpColumnBatch#getDoubleArrayElements(int, int) */ diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatch.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatch.java index 87e8dafa..20f0e82a 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatch.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatch.java @@ -25,6 +25,9 @@ package io.questdb.client.cutlass.qwp.client; import io.questdb.client.cutlass.qwp.protocol.QwpConstants; +import io.questdb.client.std.Decimal128; +import io.questdb.client.std.Decimal256; +import io.questdb.client.std.Decimal64; import io.questdb.client.std.Long256Sink; import io.questdb.client.std.ObjList; import io.questdb.client.std.Unsafe; @@ -267,6 +270,24 @@ public byte getColumnWireType(int col) { return columns.getQuick(col).wireType; } + /** + * Zero-allocation read of a DECIMAL128 value into a caller-supplied + * {@link Decimal128} sink. Sets the sink's high, low, and scale in a single + * call. Returns {@code true} on a hit, {@code false} for NULL rows (the + * sink is left untouched). + */ + public boolean getDecimal128(int col, int row, Decimal128 sink) { + QwpColumnLayout l = columnLayouts.getQuick(col); + if (isLayoutNull(l, row)) return false; + long base = l.valuesAddr + 16L * l.denseIndex(row); + sink.of( + Unsafe.getUnsafe().getLong(base + 8L), + Unsafe.getUnsafe().getLong(base), + columns.getQuick(col).scale + ); + return true; + } + /** * Returns the high 64 bits of a DECIMAL128 value. Combine with {@link #getDecimal128Low}. */ @@ -285,6 +306,40 @@ public long getDecimal128Low(int col, int row) { return Unsafe.getUnsafe().getLong(l.valuesAddr + 16L * l.denseIndex(row)); } + /** + * Zero-allocation read of a DECIMAL256 value into a caller-supplied + * {@link Decimal256} sink. Sets all four 64-bit words and the scale in a + * single call. Returns {@code true} on a hit, {@code false} for NULL rows + * (the sink is left untouched). + */ + public boolean getDecimal256(int col, int row, Decimal256 sink) { + QwpColumnLayout l = columnLayouts.getQuick(col); + if (isLayoutNull(l, row)) return false; + long base = l.valuesAddr + 32L * l.denseIndex(row); + sink.of( + Unsafe.getUnsafe().getLong(base + 24L), + Unsafe.getUnsafe().getLong(base + 16L), + Unsafe.getUnsafe().getLong(base + 8L), + Unsafe.getUnsafe().getLong(base), + columns.getQuick(col).scale + ); + return true; + } + + /** + * Zero-allocation read of a DECIMAL64 value into a caller-supplied + * {@link Decimal64} sink. Sets the unscaled value and scale in a single + * call. Returns {@code true} on a hit, {@code false} for NULL rows (the + * sink is left untouched). + */ + public boolean getDecimal64(int col, int row, Decimal64 sink) { + QwpColumnLayout l = columnLayouts.getQuick(col); + if (isLayoutNull(l, row)) return false; + long value = Unsafe.getUnsafe().getLong(l.valuesAddr + 8L * l.denseIndex(row)); + sink.of(value, columns.getQuick(col).scale); + return true; + } + /** * Scale (number of fractional digits) for DECIMAL64 / DECIMAL128 / DECIMAL256 * columns. Same across every row in the batch. Undefined for non-DECIMAL columns. diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatchHandler.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatchHandler.java index eb418b75..a4ac77ee 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatchHandler.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpColumnBatchHandler.java @@ -57,6 +57,20 @@ public interface QwpColumnBatchHandler { */ void onEnd(long totalRows); + /** + * Correlation-aware overload of {@link #onEnd(long)}. Receives the + * client-assigned request id of the just-completed query so callers can + * join completion records to earlier {@code onBatch} or log entries. + * The default delegates to {@link #onEnd(long)} for backwards + * compatibility; override this method when you need the request id. + * + * @param requestId client-assigned request id (matches {@link QwpColumnBatch#requestId()}) + * @param totalRows server-reported total row count (0 if not tracked) + */ + default void onEnd(long requestId, long totalRows) { + onEnd(totalRows); + } + /** * Invoked exactly once if the query fails at any point, instead of * {@link #onEnd} / {@link #onExecDone}. @@ -66,6 +80,26 @@ public interface QwpColumnBatchHandler { */ void onError(byte status, String message); + /** + * Correlation-aware overload of {@link #onError(byte, String)}. Receives + * the client-assigned request id of the failed query so callers can join + * the error to earlier {@code onBatch} or log entries without having to + * stash the id from a prior callback. The default delegates to + * {@link #onError(byte, String)} for backwards compatibility; override + * this method when you need the request id. + *

+ * {@code requestId} is {@code -1} when the failure is raised before a + * request was assigned -- e.g. a {@code QwpQueryClient} that is already + * closed. + * + * @param requestId client-assigned request id, or {@code -1} if no request was in flight + * @param status one of the QWP status codes (e.g., {@code STATUS_PARSE_ERROR}) + * @param message server-supplied error message (may be empty) + */ + default void onError(long requestId, byte status, String message) { + onError(status, message); + } + /** * Invoked when {@link QwpQueryClient#execute} has transparently reconnected * to another endpoint after a transport failure and is about to re-submit @@ -86,6 +120,19 @@ public interface QwpColumnBatchHandler { default void onFailoverReset(QwpServerInfo newNode) { } + /** + * Correlation-aware overload of {@link #onFailoverReset(QwpServerInfo)}. + * Receives the client-assigned request id of the query that is about to + * be re-submitted. The default delegates to + * {@link #onFailoverReset(QwpServerInfo)} for backwards compatibility. + * + * @param requestId client-assigned request id of the query being replayed + * @param newNode the endpoint just bound to, or {@code null} if the new server negotiated v1 + */ + default void onFailoverReset(long requestId, QwpServerInfo newNode) { + onFailoverReset(newNode); + } + /** * Invoked in place of {@link #onBatch} + {@link #onEnd} when the query was * a non-SELECT (DDL, INSERT, UPDATE, etc.). No batches are delivered for @@ -103,4 +150,18 @@ default void onFailoverReset(QwpServerInfo newNode) { */ default void onExecDone(short opType, long rowsAffected) { } + + /** + * Correlation-aware overload of {@link #onExecDone(short, long)}. Receives + * the client-assigned request id of the completed non-SELECT query. The + * default delegates to {@link #onExecDone(short, long)} for backwards + * compatibility. + * + * @param requestId client-assigned request id (matches {@link QwpColumnBatch#requestId()}) + * @param opType server-side opType constant (CompiledQuery.SELECT / INSERT / UPDATE / CREATE_TABLE / etc.) + * @param rowsAffected rows inserted / updated / deleted; 0 for pure DDL + */ + default void onExecDone(long requestId, short opType, long rowsAffected) { + onExecDone(opType, rowsAffected); + } } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java index c1c7a1ff..64f0dfd5 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java @@ -1629,12 +1629,12 @@ private void executeImpl(String sql, QwpBindSetter binds, QwpColumnBatchHandler return; } if (!failoverEnabled) { - handler.onError(probe.interceptedStatus, probe.interceptedMessage); + handler.onError(probe.interceptedRequestId, probe.interceptedStatus, probe.interceptedMessage); return; } if (attempt >= failoverMaxAttempts || System.nanoTime() - failoverDeadlineNanos >= 0) { int failovers = Math.max(0, attempt - 1); - handler.onError(probe.interceptedStatus, + handler.onError(probe.interceptedRequestId, probe.interceptedStatus, "transport failure after " + attempt + " execute attempt" + (attempt == 1 ? "" : "s") + " (" + failovers + " failover reconnect" @@ -1664,7 +1664,7 @@ private void executeImpl(String sql, QwpBindSetter binds, QwpColumnBatchHandler long remaining = remainingNanos <= 0L ? 0L : remainingNanos / 1_000_000L; if (remainingNanos <= 0L) { int failovers = Math.max(0, attempt - 1); - handler.onError(probe.interceptedStatus, + handler.onError(probe.interceptedRequestId, probe.interceptedStatus, "transport failure after " + attempt + " execute attempt" + (attempt == 1 ? "" : "s") + " (" + failovers + " failover reconnect" @@ -1680,7 +1680,7 @@ private void executeImpl(String sql, QwpBindSetter binds, QwpColumnBatchHandler Thread.sleep(delay); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); - handler.onError(probe.interceptedStatus, + handler.onError(probe.interceptedRequestId, probe.interceptedStatus, "failover interrupted while backing off after attempt " + attempt + "; last error: " + probe.interceptedMessage); return; @@ -1694,21 +1694,21 @@ private void executeImpl(String sql, QwpBindSetter binds, QwpColumnBatchHandler // Credentials are cluster-wide, so retrying floods server logs // without recovery. Surface a distinct message so monitoring // can pull auth incidents apart from generic transport failures. - handler.onError(probe.interceptedStatus, + handler.onError(probe.interceptedRequestId, probe.interceptedStatus, "auth failure during failover reconnect [host=" + authErr.getHost() + ':' + authErr.getPort() + ", status=" + authErr.getStatusCode() + ", last error: " + probe.interceptedMessage + ']'); return; } catch (RuntimeException reconnectErr) { - handler.onError(probe.interceptedStatus, + handler.onError(probe.interceptedRequestId, probe.interceptedStatus, "failover reconnect failed after " + attempt + " attempt" + (attempt == 1 ? "" : "s") + " [last error: " + probe.interceptedMessage + ", reconnect error: " + reconnectErr.getMessage() + ']'); return; } - handler.onFailoverReset(serverInfo); + handler.onFailoverReset(probe.interceptedRequestId, serverInfo); } } @@ -1725,13 +1725,13 @@ private void executeOnce(String sql, QwpBindSetter binds, FailoverProbeHandler p // before close() returns. QwpEgressIoThread io = ioThread; if (io == null) { - probe.onError(WebSocketResponse.STATUS_INTERNAL_ERROR, "QwpQueryClient is closed"); + probe.onError(-1L, WebSocketResponse.STATUS_INTERNAL_ERROR, "QwpQueryClient is closed"); return; } GenerationListener listener = currentGenerationListener; TerminalFailure tf = listener != null ? listener.get() : null; if (tf != null) { - probe.markTransportFailure(tf.status, tf.message); + probe.markTransportFailure(-1L, tf.status, tf.message); return; } bindValues.reset(); @@ -1746,7 +1746,7 @@ private void executeOnce(String sql, QwpBindSetter binds, FailoverProbeHandler p // transport failures -- they're deterministic on the user thread -- // so they must not trigger failover. bindValues.reset(); - probe.deliverFinal( + probe.deliverFinal(-1L, "bind encoding failed: " + e.getMessage()); return; } @@ -1767,19 +1767,19 @@ private void executeOnce(String sql, QwpBindSetter binds, FailoverProbeHandler p } break; case QueryEvent.KIND_END: - probe.onEnd(ev.totalRows); + probe.onEnd(requestId, ev.totalRows); return; case QueryEvent.KIND_EXEC_DONE: - probe.onExecDone(ev.opType, ev.rowsAffected); + probe.onExecDone(requestId, ev.opType, ev.rowsAffected); return; case QueryEvent.KIND_ERROR: - probe.onError(ev.errorStatus, ev.errorMessage); + probe.onError(requestId, ev.errorStatus, ev.errorMessage); return; case QueryEvent.KIND_TRANSPORT_ERROR: - probe.markTransportFailure(ev.errorStatus, ev.errorMessage); + probe.markTransportFailure(requestId, ev.errorStatus, ev.errorMessage); return; default: - probe.onError(WebSocketResponse.STATUS_INTERNAL_ERROR, "unknown event kind " + ev.kind); + probe.onError(requestId, WebSocketResponse.STATUS_INTERNAL_ERROR, "unknown event kind " + ev.kind); return; } } finally { @@ -1793,7 +1793,7 @@ private void executeOnce(String sql, QwpBindSetter binds, FailoverProbeHandler p } catch (InterruptedException ie) { Thread.currentThread().interrupt(); // Interrupt on the user thread is not a transport failure; surface directly. - probe.deliverFinal("interrupted while waiting for server response"); + probe.deliverFinal(requestId, "interrupted while waiting for server response"); } finally { currentRequestId = -1L; } @@ -2021,6 +2021,7 @@ private static final class Endpoint { private static final class FailoverProbeHandler implements QwpColumnBatchHandler { final QwpColumnBatchHandler delegate; String interceptedMessage; + long interceptedRequestId = -1L; byte interceptedStatus; boolean transportFailureIntercepted; @@ -2038,6 +2039,11 @@ public void onEnd(long totalRows) { delegate.onEnd(totalRows); } + @Override + public void onEnd(long requestId, long totalRows) { + delegate.onEnd(requestId, totalRows); + } + @Override public void onError(byte status, String message) { // Server-emitted QUERY_ERROR. Pass straight through. Transport @@ -2045,18 +2051,38 @@ public void onError(byte status, String message) { delegate.onError(status, message); } + @Override + public void onError(long requestId, byte status, String message) { + delegate.onError(requestId, status, message); + } + @Override public void onExecDone(short opType, long rowsAffected) { delegate.onExecDone(opType, rowsAffected); } + @Override + public void onExecDone(long requestId, short opType, long rowsAffected) { + delegate.onExecDone(requestId, opType, rowsAffected); + } + + @Override + public void onFailoverReset(QwpServerInfo newNode) { + delegate.onFailoverReset(newNode); + } + + @Override + public void onFailoverReset(long requestId, QwpServerInfo newNode) { + delegate.onFailoverReset(requestId, newNode); + } + /** * Bypass the interception logic and deliver the error straight to the * user. Used for failures that are not transport-related (bind-encode * errors, interrupts) so they don't trigger a spurious failover. */ - void deliverFinal(String message) { - delegate.onError(WebSocketResponse.STATUS_INTERNAL_ERROR, message); + void deliverFinal(long requestId, String message) { + delegate.onError(requestId, WebSocketResponse.STATUS_INTERNAL_ERROR, message); } /** @@ -2066,8 +2092,9 @@ void deliverFinal(String message) { * (failover=on + reconnect succeeds) or a single final {@code onError} * (failover=off or reconnect exhausted). */ - void markTransportFailure(byte status, String message) { + void markTransportFailure(long requestId, byte status, String message) { transportFailureIntercepted = true; + interceptedRequestId = requestId; interceptedStatus = status; interceptedMessage = message; } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java index fbf66d47..f59dcb4b 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java @@ -39,6 +39,7 @@ import io.questdb.client.cutlass.line.LineSenderException; import io.questdb.client.cutlass.line.array.DoubleArray; import io.questdb.client.cutlass.line.array.LongArray; +import io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerPool; import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop; import io.questdb.client.cutlass.qwp.client.sf.cursor.DefaultSenderConnectionListener; @@ -66,7 +67,6 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -196,8 +196,7 @@ public class QwpWebSocketSender implements Sender { // Orphan-slot drainer pool. Non-null only when the builder requested // drain_orphans=true AND we have a slot path to scan against. Closed // alongside the cursor send loop in close(). - private io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerPool - drainerPool; + private BackgroundDrainerPool drainerPool; // Keepalive PING cadence used by the I/O loop while // request_durable_ack=on AND there are pending durable-ack // confirmations. Default mirrors the loop's spec value; 0 or negative @@ -281,7 +280,7 @@ private QwpWebSocketSender( if (endpoints == null || endpoints.isEmpty()) { throw new IllegalArgumentException("endpoints must be non-empty"); } - this.endpoints = Collections.unmodifiableList(new ArrayList<>(endpoints)); + this.endpoints = List.copyOf(endpoints); this.hostTracker = new QwpHostHealthTracker(this.endpoints.size()); this.authorizationHeader = authorizationHeader; this.tlsConfig = tlsConfig; @@ -770,6 +769,7 @@ public void atNow() { * @return {@code true} if {@code ackedFsn() >= targetFsn} on return, {@code false} on timeout * @throws LineSenderException if the I/O loop has latched a terminal error */ + @Override public boolean awaitAckedFsn(long targetFsn, long timeoutMillis) { checkNotClosed(); if (cursorEngine == null) { @@ -1360,6 +1360,7 @@ public void flush() { * * @return highest FSN published into the engine, or {@code -1} if no data */ + @Override public long flushAndGetSequence() { checkNotClosed(); ensureNoInProgressRow(); @@ -1459,6 +1460,7 @@ public QwpWebSocketSender geoHashColumn(CharSequence columnName, CharSequence va * Snapshot accessor — for a bounded wait, use * {@link #awaitAckedFsn(long, long)}. */ + @Override public long getAckedFsn() { return cursorEngine != null ? cursorEngine.ackedFsn() : -1L; } @@ -1471,7 +1473,7 @@ public long getAckedFsn() { * moments ago may still count for a few ms. */ public int getActiveBackgroundDrainers() { - io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerPool pool = drainerPool; + BackgroundDrainerPool pool = drainerPool; return pool == null ? 0 : pool.getActiveCount(); } @@ -1496,6 +1498,28 @@ public int getAutoFlushRows() { return autoFlushRows; } + /** + * Number of {@link SenderConnectionEvent} notifications dropped because + * the bounded inbox was full. Non-zero means the user-supplied + * {@link SenderConnectionListener} cannot keep up. Returns 0 if the + * dispatcher has not been allocated yet. + */ + public long getDroppedConnectionNotifications() { + SenderConnectionDispatcher d = connectionDispatcher; + return d == null ? 0L : d.getDroppedNotifications(); + } + + /** + * Number of {@link SenderError} notifications dropped because the + * bounded inbox was full. Non-zero means the user-supplied + * {@link SenderErrorHandler} cannot keep up. Returns 0 if the error + * dispatcher has not been allocated yet. + */ + public long getDroppedErrorNotifications() { + SenderErrorDispatcher d = errorDispatcher; + return d == null ? 0L : d.getDroppedNotifications(); + } + /** * Snapshot of the typed payload for the latched terminal server-rejection error, * or {@code null} if the I/O loop has not latched a server-rejection terminal @@ -1535,38 +1559,6 @@ public QwpTableBuffer getTableBuffer(String tableName) { return buffer; } - /** - * Number of {@link SenderConnectionEvent} notifications dropped because - * the bounded inbox was full. Non-zero means the user-supplied - * {@link SenderConnectionListener} cannot keep up. Returns 0 if the - * dispatcher has not been allocated yet. - */ - public long getDroppedConnectionNotifications() { - SenderConnectionDispatcher d = connectionDispatcher; - return d == null ? 0L : d.getDroppedNotifications(); - } - - /** - * Number of {@link SenderError} notifications dropped because the - * bounded inbox was full. Non-zero means the user-supplied - * {@link SenderErrorHandler} cannot keep up. Returns 0 if the error - * dispatcher has not been allocated yet. - */ - public long getDroppedErrorNotifications() { - SenderErrorDispatcher d = errorDispatcher; - return d == null ? 0L : d.getDroppedNotifications(); - } - - /** - * Number of {@link SenderConnectionEvent} notifications delivered to the - * user listener since this sender started. Counts every delivery attempt, - * including those where the listener threw. - */ - public long getTotalConnectionEventsDelivered() { - SenderConnectionDispatcher d = connectionDispatcher; - return d == null ? 0L : d.getTotalDelivered(); - } - /** * Total binary frames whose ACKs have been received and applied. */ @@ -1582,7 +1574,7 @@ public long getTotalAcks() { * slots were discovered at startup. */ public long getTotalBackgroundDrainersFailed() { - io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerPool pool = drainerPool; + BackgroundDrainerPool pool = drainerPool; return pool == null ? 0L : pool.getTotalFailed(); } @@ -1593,7 +1585,7 @@ public long getTotalBackgroundDrainersFailed() { * slots were discovered at startup. */ public long getTotalBackgroundDrainersSucceeded() { - io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainerPool pool = drainerPool; + BackgroundDrainerPool pool = drainerPool; return pool == null ? 0L : pool.getTotalSucceeded(); } @@ -1608,6 +1600,16 @@ public long getTotalBackpressureStalls() { return e == null ? 0L : e.getTotalBackpressureStalls(); } + /** + * Number of {@link SenderConnectionEvent} notifications delivered to the + * user listener since this sender started. Counts every delivery attempt, + * including those where the listener threw. + */ + public long getTotalConnectionEventsDelivered() { + SenderConnectionDispatcher d = connectionDispatcher; + return d == null ? 0L : d.getTotalDelivered(); + } + /** * Number of {@link SenderError} notifications delivered to the user * handler since this sender started. Counts every delivery attempt, @@ -1911,22 +1913,6 @@ public void reset() { cachedTimestampNanosColumn = null; } - /** - * Attach a {@link CursorSendEngine} for store-and-forward. Must be called - * before the first send. - */ - public void setCursorEngine(CursorSendEngine engine, boolean takeOwnership) { - if (closed) { - throw new LineSenderException("Sender is closed"); - } - if (connected) { - throw new LineSenderException( - "setCursorEngine must be called before the first send"); - } - this.cursorEngine = engine; - this.ownsCursorEngine = takeOwnership && engine != null; - } - /** * Register an async listener for connection-state transitions: initial * connect, primary failover, endpoint attempt failures, the full address @@ -1963,6 +1949,22 @@ public void setConnectionListenerInboxCapacity(int capacity) { this.connectionListenerInboxCapacity = capacity; } + /** + * Attach a {@link CursorSendEngine} for store-and-forward. Must be called + * before the first send. + */ + public void setCursorEngine(CursorSendEngine engine, boolean takeOwnership) { + if (closed) { + throw new LineSenderException("Sender is closed"); + } + if (connected) { + throw new LineSenderException( + "setCursorEngine must be called before the first send"); + } + this.cursorEngine = engine; + this.ownsCursorEngine = takeOwnership && engine != null; + } + /** * Configure the user-supplied error handler. May be called either before * or after {@code connect()} — when called after, the change propagates @@ -2260,26 +2262,6 @@ private static List singleEndpoint(String host, int port) { return Collections.singletonList(new Endpoint(host, port)); } - private void atMicros(long timestampMicros) { - // Add designated timestamp column (empty name for designated timestamp) - // Use cached reference to avoid hashmap lookup per row - if (cachedTimestampColumn == null) { - cachedTimestampColumn = currentTableBuffer.getOrCreateDesignatedTimestampColumn(QwpConstants.TYPE_TIMESTAMP); - } - cachedTimestampColumn.addLong(timestampMicros); - sendRow(); - } - - private void atNanos(long timestampNanos) { - // Add designated timestamp column (empty name for designated timestamp) - // Use cached reference to avoid hashmap lookup per row - if (cachedTimestampNanosColumn == null) { - cachedTimestampNanosColumn = currentTableBuffer.getOrCreateDesignatedTimestampColumn(QwpConstants.TYPE_TIMESTAMP_NANOS); - } - cachedTimestampNanosColumn.addLong(timestampNanos); - sendRow(); - } - /** * Clamps the soft-flush byte budget to fit under the server's advertised * X-QWP-Max-Batch-Size minus a safety margin for encoding overhead @@ -2320,6 +2302,26 @@ private void applyServerBatchSizeLimit(int advertisedMaxBatchSize) { } } + private void atMicros(long timestampMicros) { + // Add designated timestamp column (empty name for designated timestamp) + // Use cached reference to avoid hashmap lookup per row + if (cachedTimestampColumn == null) { + cachedTimestampColumn = currentTableBuffer.getOrCreateDesignatedTimestampColumn(QwpConstants.TYPE_TIMESTAMP); + } + cachedTimestampColumn.addLong(timestampMicros); + sendRow(); + } + + private void atNanos(long timestampNanos) { + // Add designated timestamp column (empty name for designated timestamp) + // Use cached reference to avoid hashmap lookup per row + if (cachedTimestampNanosColumn == null) { + cachedTimestampNanosColumn = currentTableBuffer.getOrCreateDesignatedTimestampColumn(QwpConstants.TYPE_TIMESTAMP_NANOS); + } + cachedTimestampNanosColumn.addLong(timestampNanos); + sendRow(); + } + private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) { int previousIdx = ctx.previousIdx; if (previousIdx >= 0) { @@ -2403,7 +2405,7 @@ private synchronized WebSocketClient buildAndConnect(ReconnectSupplier ctx) { if (terminalUpgradeError == null && ( classified instanceof QwpVersionMismatchException || (classified instanceof WebSocketUpgradeException - && !((WebSocketUpgradeException) classified).isRoleMismatch()))) { + && !((WebSocketUpgradeException) classified).isRoleMismatch()))) { terminalUpgradeError = classified; } hostTracker.recordTransportError(idx); @@ -2897,7 +2899,6 @@ private void flushPendingRows() { // re-batch with fewer rows per flush. close()'s drain path // also relies on this being a clean reset to avoid re-throwing. if (serverMaxBatchSize > 0 && messageSize > serverMaxBatchSize) { - int oversize = messageSize; int droppedRows = pendingRowCount; for (int i = 0, n = keys.size(); i < n; i++) { CharSequence tableName = keys.getQuick(i); @@ -2916,7 +2917,7 @@ private void flushPendingRows() { pendingRowCount = 0; firstPendingRowTimeNanos = 0; throw new LineSenderException("batch too large for server batch cap") - .put(" [messageSize=").put(oversize) + .put(" [messageSize=").put(messageSize) .put(", serverMaxBatchSize=").put(serverMaxBatchSize) .put(", droppedRows=").put(droppedRows).put(']'); } @@ -3132,22 +3133,6 @@ private long toMicros(long value, ChronoUnit unit) { } } - private long totalBufferedBytes() { - long total = 0; - ObjList keys = tableBuffers.keys(); - for (int i = 0, n = keys.size(); i < n; i++) { - CharSequence tableName = keys.getQuick(i); - if (tableName == null) { - continue; - } - QwpTableBuffer tb = tableBuffers.get(tableName); - if (tb != null) { - total += tb.getBufferedBytes(); - } - } - return total; - } - private void validateTableName(CharSequence name) { if (name == null || !TableUtils.isValidTableName(name, MAX_TABLE_NAME_LENGTH)) { if (name == null || name.length() == 0) { diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/RowView.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/RowView.java index 6405374b..13ec5add 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/RowView.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/RowView.java @@ -24,6 +24,9 @@ package io.questdb.client.cutlass.qwp.client; +import io.questdb.client.std.Decimal128; +import io.questdb.client.std.Decimal256; +import io.questdb.client.std.Decimal64; import io.questdb.client.std.Long256Sink; import io.questdb.client.std.Uuid; import io.questdb.client.std.bytes.DirectByteSequence; @@ -124,6 +127,13 @@ public char getCharValue(int col) { return batch.getCharValue(col, row); } + /** + * @see QwpColumnBatch#getDecimal128(int, int, Decimal128) + */ + public boolean getDecimal128(int col, Decimal128 sink) { + return batch.getDecimal128(col, row, sink); + } + /** * @see QwpColumnBatch#getDecimal128High(int, int) */ @@ -138,6 +148,20 @@ public long getDecimal128Low(int col) { return batch.getDecimal128Low(col, row); } + /** + * @see QwpColumnBatch#getDecimal256(int, int, Decimal256) + */ + public boolean getDecimal256(int col, Decimal256 sink) { + return batch.getDecimal256(col, row, sink); + } + + /** + * @see QwpColumnBatch#getDecimal64(int, int, Decimal64) + */ + public boolean getDecimal64(int col, Decimal64 sink) { + return batch.getDecimal64(col, row, sink); + } + /** * @see QwpColumnBatch#getDoubleArrayElements(int, int) */ diff --git a/core/src/main/java/io/questdb/client/impl/ConfigStringTranslator.java b/core/src/main/java/io/questdb/client/impl/ConfigStringTranslator.java new file mode 100644 index 00000000..5888a775 --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/ConfigStringTranslator.java @@ -0,0 +1,376 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.std.Chars; +import io.questdb.client.std.str.StringSink; + +/** + * Translates a unified configuration string into the three things needed to + * build a {@code QuestDB}: an ingest-side config (Sender), an egress-side + * config (QwpQueryClient), and an optional pool-tuning bundle. + *

+ * Pool-tuning keys are stripped from the connection-config + * strings (neither downstream parser accepts them) and surfaced separately + * via {@link PoolConfig}: + *

+ *

+ * Schema translation: http<->ws, https<->wss. + * A curated subset of keys carries over to the derived side (addr, token / + * auth, TLS); everything else stays on the input side only. + *

+ * The parser runs once at {@code QuestDB.connect(...)} time. Allocation here + * is one-shot startup cost; the hot borrow / submit paths never see it. + */ +public final class ConfigStringTranslator { + + private ConfigStringTranslator() { + } + + /** + * Returns the ingest and query configuration strings plus the pool config + * extracted from a unified input. + */ + public static Bundle deriveBothSides(CharSequence config) { + if (config == null || config.length() == 0) { + throw new IllegalArgumentException("configuration string cannot be empty"); + } + StringSink sink = new StringSink(); + int pos = ConfStringParser.of(config, sink); + if (pos < 0) { + throw new IllegalArgumentException("invalid configuration string: " + sink); + } + boolean isHttp; + boolean isTls; + if (Chars.equals("http", sink)) { + isHttp = true; + isTls = false; + } else if (Chars.equals("https", sink)) { + isHttp = true; + isTls = true; + } else if (Chars.equals("ws", sink)) { + isHttp = false; + isTls = false; + } else if (Chars.equals("wss", sink)) { + isHttp = false; + isTls = true; + } else { + throw new IllegalArgumentException( + "QuestDB.connect(single config) supports schemas [http, https, ws, wss]; got: " + sink + + ". Use QuestDB.connect(ingestConfig, queryConfig) for other transports."); + } + + // Curated keys are mirrored to the derived side too. + StringSink addr = new StringSink(); + StringSink token = new StringSink(); + StringSink username = new StringSink(); + StringSink password = new StringSink(); + StringSink auth = new StringSink(); + StringSink tlsRoots = new StringSink(); + StringSink tlsRootsPassword = new StringSink(); + StringSink tlsVerify = new StringSink(); + boolean hasAddr = false; + boolean hasToken = false; + boolean hasUsername = false; + boolean hasPassword = false; + boolean hasAuth = false; + boolean hasTlsRoots = false; + boolean hasTlsRootsPassword = false; + boolean hasTlsVerify = false; + + // Input-side passthrough: schema:: + every non-pool key encountered. + // We always re-serialize rather than pass the raw string through, so + // pool keys can be stripped cleanly even when they sit between two + // unrelated keys. + StringSink inputPassthrough = new StringSink(); + inputPassthrough.put(isHttp ? (isTls ? "https::" : "http::") : (isTls ? "wss::" : "ws::")); + + PoolConfig poolConfig = new PoolConfig(); + + while (ConfStringParser.hasNext(config, pos)) { + pos = ConfStringParser.nextKey(config, pos, sink); + if (pos < 0) { + throw new IllegalArgumentException("invalid configuration string: " + sink); + } + String key = sink.toString(); + pos = ConfStringParser.value(config, pos, sink); + if (pos < 0) { + throw new IllegalArgumentException("invalid configuration string: " + sink); + } + // First, try to consume as a pool key. If matched, do NOT echo to + // the passthrough (downstream parsers reject these). + if (consumePoolKey(key, sink, poolConfig)) { + continue; + } + // Capture curated keys for the derived-side rebuild, but also echo + // them to the input-side passthrough (the matching parser still + // needs to see them). + switch (key) { + case "addr": + addr.clear(); + addr.put(sink); + hasAddr = true; + break; + case "token": + token.clear(); + token.put(sink); + hasToken = true; + break; + case "username": + username.clear(); + username.put(sink); + hasUsername = true; + break; + case "password": + password.clear(); + password.put(sink); + hasPassword = true; + break; + case "auth": + auth.clear(); + auth.put(sink); + hasAuth = true; + break; + case "tls_roots": + tlsRoots.clear(); + tlsRoots.put(sink); + hasTlsRoots = true; + break; + case "tls_roots_password": + tlsRootsPassword.clear(); + tlsRootsPassword.put(sink); + hasTlsRootsPassword = true; + break; + case "tls_verify": + tlsVerify.clear(); + tlsVerify.put(sink); + hasTlsVerify = true; + break; + default: + break; + } + appendKv(inputPassthrough, key, sink); + } + if (!hasAddr) { + throw new IllegalArgumentException("configuration string is missing 'addr'"); + } + + String ingest; + String query; + if (isHttp) { + ingest = inputPassthrough.toString(); + query = buildQueryConfig(isTls, addr, hasToken, token, hasUsername, + hasPassword, hasAuth, auth, + hasTlsRoots, tlsRoots, hasTlsRootsPassword, tlsRootsPassword, + hasTlsVerify, tlsVerify); + } else { + query = inputPassthrough.toString(); + ingest = buildIngestConfig(isTls, addr, hasToken, token, hasUsername, username, + hasPassword, password, hasAuth, auth, + hasTlsRoots, tlsRoots, hasTlsRootsPassword, tlsRootsPassword, + hasTlsVerify, tlsVerify); + } + return new Bundle(ingest, query, poolConfig); + } + + private static void appendKv(StringSink out, String key, CharSequence value) { + out.put(key).put('='); + // Values may contain ';' which must be doubled (per ConfStringParser). + for (int i = 0, n = value.length(); i < n; i++) { + char c = value.charAt(i); + out.put(c); + if (c == ';') { + out.put(';'); + } + } + out.put(';'); + } + + private static String buildIngestConfig( + boolean isTls, + CharSequence addr, + boolean hasToken, CharSequence token, + boolean hasUsername, CharSequence username, + boolean hasPassword, CharSequence password, + boolean hasAuth, CharSequence auth, + boolean hasTlsRoots, CharSequence tlsRoots, + boolean hasTlsRootsPassword, CharSequence tlsRootsPassword, + boolean hasTlsVerify, CharSequence tlsVerify + ) { + StringSink out = new StringSink(); + out.put(isTls ? "https::" : "http::"); + appendKv(out, "addr", addr); + if (hasToken) { + appendKv(out, "token", token); + } + if (hasUsername) { + appendKv(out, "username", username); + } + if (hasPassword) { + appendKv(out, "password", password); + } + if (hasAuth && !hasToken && !hasUsername) { + appendKv(out, "auth", auth); + } + if (hasTlsRoots) { + appendKv(out, "tls_roots", tlsRoots); + } + if (hasTlsRootsPassword) { + appendKv(out, "tls_roots_password", tlsRootsPassword); + } + if (hasTlsVerify) { + appendKv(out, "tls_verify", tlsVerify); + } + return out.toString(); + } + + private static String buildQueryConfig( + boolean isTls, + CharSequence addr, + boolean hasToken, CharSequence token, + boolean hasUsername, + boolean hasPassword, + boolean hasAuth, CharSequence auth, + boolean hasTlsRoots, CharSequence tlsRoots, + boolean hasTlsRootsPassword, CharSequence tlsRootsPassword, + boolean hasTlsVerify, CharSequence tlsVerify + ) { + StringSink out = new StringSink(); + out.put(isTls ? "wss::" : "ws::"); + appendKv(out, "addr", addr); + if (hasAuth) { + appendKv(out, "auth", auth); + } else if (hasToken) { + StringSink bearer = new StringSink(); + bearer.put("Bearer ").put(token); + appendKv(out, "auth", bearer); + } else if (hasUsername && hasPassword) { + throw new IllegalArgumentException( + "username/password auth is not supported in unified config for ws/wss derivation; " + + "pass auth=Basic directly, or use the builder with explicit queryConfig()"); + } + if (isTls) { + if (hasTlsRoots) { + appendKv(out, "tls_roots", tlsRoots); + } + if (hasTlsRootsPassword) { + appendKv(out, "tls_roots_password", tlsRootsPassword); + } + if (hasTlsVerify) { + appendKv(out, "tls_verify", tlsVerify); + } + } + return out.toString(); + } + + private static boolean consumePoolKey(String key, CharSequence value, PoolConfig out) { + switch (key) { + case "sender_pool_min": + out.senderPoolMin = parseInt(key, value); + return true; + case "sender_pool_max": + out.senderPoolMax = parseInt(key, value); + return true; + case "query_pool_min": + out.queryPoolMin = parseInt(key, value); + return true; + case "query_pool_max": + out.queryPoolMax = parseInt(key, value); + return true; + case "acquire_timeout_ms": + out.acquireTimeoutMillis = parseLong(key, value); + return true; + case "idle_timeout_ms": + out.idleTimeoutMillis = parseLong(key, value); + return true; + case "max_lifetime_ms": + out.maxLifetimeMillis = parseLong(key, value); + return true; + case "housekeeper_interval_ms": + out.housekeeperIntervalMillis = parseLong(key, value); + return true; + default: + return false; + } + } + + private static int parseInt(String key, CharSequence value) { + try { + return Integer.parseInt(value.toString()); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("invalid " + key + ": " + value); + } + } + + private static long parseLong(String key, CharSequence value) { + try { + return Long.parseLong(value.toString()); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("invalid " + key + ": " + value); + } + } + + /** + * The full result of translating a single connect string: an ingest-side + * config, an egress-side config, and any pool-tuning values that the + * string carried (or all-unset {@link PoolConfig} if it carried none). + */ + public static final class Bundle { + public final String ingestConfig; + public final PoolConfig poolConfig; + public final String queryConfig; + + Bundle(String ingestConfig, String queryConfig, PoolConfig poolConfig) { + this.ingestConfig = ingestConfig; + this.queryConfig = queryConfig; + this.poolConfig = poolConfig; + } + } + + /** + * Pool tuning extracted from the connect string. Each field starts at + * {@link #UNSET} (-1); the builder applies only those that were actually + * present in the string, leaving the rest at the builder defaults. + */ + public static final class PoolConfig { + public static final long UNSET = -1L; + + public long acquireTimeoutMillis = UNSET; + public long housekeeperIntervalMillis = UNSET; + public long idleTimeoutMillis = UNSET; + public long maxLifetimeMillis = UNSET; + public int queryPoolMax = (int) UNSET; + public int queryPoolMin = (int) UNSET; + public int senderPoolMax = (int) UNSET; + public int senderPoolMin = (int) UNSET; + } +} diff --git a/core/src/main/java/io/questdb/client/impl/PoolHousekeeper.java b/core/src/main/java/io/questdb/client/impl/PoolHousekeeper.java new file mode 100644 index 00000000..ecdd6fc9 --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/PoolHousekeeper.java @@ -0,0 +1,92 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +/** + * Daemon thread that periodically asks both pools to reap idle / over-age + * slots. Owned by {@link QuestDBImpl}; one instance per {@code QuestDB} + * handle. + */ +final class PoolHousekeeper { + + private final long intervalMillis; + private final QueryClientPool queryPool; + private final SenderPool senderPool; + private final Object signalLock = new Object(); + private final Thread thread; + private volatile boolean stop; + + PoolHousekeeper(SenderPool senderPool, QueryClientPool queryPool, long intervalMillis) { + this.senderPool = senderPool; + this.queryPool = queryPool; + this.intervalMillis = intervalMillis; + this.thread = new Thread(this::runLoop, "questdb-pool-housekeeper"); + this.thread.setDaemon(true); + } + + void start() { + thread.start(); + } + + void stop() { + stop = true; + synchronized (signalLock) { + signalLock.notifyAll(); + } + try { + thread.join(2_000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private void runLoop() { + while (!stop) { + synchronized (signalLock) { + if (stop) { + return; + } + try { + signalLock.wait(intervalMillis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } + if (stop) { + return; + } + try { + senderPool.reapIdle(); + } catch (RuntimeException ignored) { + // Reaping must not propagate -- it's best-effort housekeeping. + } + try { + queryPool.reapIdle(); + } catch (RuntimeException ignored) { + } + } + } +} diff --git a/core/src/main/java/io/questdb/client/impl/PooledSender.java b/core/src/main/java/io/questdb/client/impl/PooledSender.java new file mode 100644 index 00000000..9346967c --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/PooledSender.java @@ -0,0 +1,360 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.Sender; +import io.questdb.client.cutlass.line.array.DoubleArray; +import io.questdb.client.cutlass.line.array.LongArray; +import io.questdb.client.std.Decimal128; +import io.questdb.client.std.Decimal256; +import io.questdb.client.std.Decimal64; +import io.questdb.client.std.bytes.DirectByteSlice; +import org.jetbrains.annotations.NotNull; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; + +/** + * Decorator that lends a real {@link Sender} from {@link SenderPool}. The + * decorator is pre-allocated once per pool slot and reused for every borrow. + *

+ * Behavior difference from a raw Sender: {@link #close()} on a pooled Sender + * flushes the buffer and returns the decorator to the pool. The underlying + * Sender is only truly closed when {@link io.questdb.client.QuestDB#close()} + * shuts down the pool. + */ +public final class PooledSender implements Sender { + + private final long createdAtMillis; + private final Sender delegate; + private final SenderPool pool; + private volatile long idleSinceMillis; + private volatile boolean inUse; + + PooledSender(Sender delegate, SenderPool pool) { + this.delegate = delegate; + this.pool = pool; + this.createdAtMillis = System.currentTimeMillis(); + this.idleSinceMillis = this.createdAtMillis; + } + + @Override + public void at(long timestamp, ChronoUnit unit) { + delegate.at(timestamp, unit); + } + + @Override + public void at(Instant timestamp) { + delegate.at(timestamp); + } + + @Override + public void atNow() { + delegate.atNow(); + } + + @Override + public boolean awaitAckedFsn(long targetFsn, long timeoutMillis) { + return delegate.awaitAckedFsn(targetFsn, timeoutMillis); + } + + @Override + public Sender binaryColumn(CharSequence name, byte[] value) { + delegate.binaryColumn(name, value); + return this; + } + + @Override + public Sender binaryColumn(CharSequence name, long ptr, long len) { + delegate.binaryColumn(name, ptr, len); + return this; + } + + @Override + public Sender binaryColumn(CharSequence name, DirectByteSlice slice) { + delegate.binaryColumn(name, slice); + return this; + } + + @Override + public Sender boolColumn(CharSequence name, boolean value) { + delegate.boolColumn(name, value); + return this; + } + + @Override + public DirectByteSlice bufferView() { + return delegate.bufferView(); + } + + @Override + public Sender byteColumn(CharSequence name, byte value) { + delegate.byteColumn(name, value); + return this; + } + + @Override + public void cancelRow() { + delegate.cancelRow(); + } + + @Override + public Sender charColumn(CharSequence name, char value) { + delegate.charColumn(name, value); + return this; + } + + /** + * Flushes pending rows and returns this decorator to the pool. Does not + * actually close the underlying {@link Sender}; that only happens when + * the owning {@code QuestDB} is closed. + *

+ * Idempotent: a second call after a return is a no-op. + */ + @Override + public void close() { + if (!inUse) { + return; + } + try { + delegate.flush(); + } finally { + inUse = false; + pool.giveBack(this); + } + } + + @Override + public Sender decimalColumn(CharSequence name, Decimal256 value) { + delegate.decimalColumn(name, value); + return this; + } + + @Override + public Sender decimalColumn(CharSequence name, Decimal128 value) { + delegate.decimalColumn(name, value); + return this; + } + + @Override + public Sender decimalColumn(CharSequence name, Decimal64 value) { + delegate.decimalColumn(name, value); + return this; + } + + @Override + public Sender decimalColumn(CharSequence name, CharSequence value) { + delegate.decimalColumn(name, value); + return this; + } + + @Override + public Sender doubleArray(@NotNull CharSequence name, double[] values) { + delegate.doubleArray(name, values); + return this; + } + + @Override + public Sender doubleArray(@NotNull CharSequence name, double[][] values) { + delegate.doubleArray(name, values); + return this; + } + + @Override + public Sender doubleArray(@NotNull CharSequence name, double[][][] values) { + delegate.doubleArray(name, values); + return this; + } + + @Override + public Sender doubleArray(CharSequence name, DoubleArray array) { + delegate.doubleArray(name, array); + return this; + } + + @Override + public Sender doubleColumn(CharSequence name, double value) { + delegate.doubleColumn(name, value); + return this; + } + + @Override + public Sender floatColumn(CharSequence name, float value) { + delegate.floatColumn(name, value); + return this; + } + + @Override + public void flush() { + delegate.flush(); + } + + @Override + public long flushAndGetSequence() { + return delegate.flushAndGetSequence(); + } + + @Override + public Sender geoHashColumn(CharSequence name, long bits, int precisionBits) { + delegate.geoHashColumn(name, bits, precisionBits); + return this; + } + + @Override + public Sender geoHashColumn(CharSequence name, CharSequence value) { + delegate.geoHashColumn(name, value); + return this; + } + + @Override + public long getAckedFsn() { + return delegate.getAckedFsn(); + } + + @Override + public Sender intColumn(CharSequence name, int value) { + delegate.intColumn(name, value); + return this; + } + + @Override + public Sender ipv4Column(CharSequence name, int address) { + delegate.ipv4Column(name, address); + return this; + } + + @Override + public Sender ipv4Column(CharSequence name, CharSequence address) { + delegate.ipv4Column(name, address); + return this; + } + + @Override + public Sender long256Column(CharSequence name, long l0, long l1, long l2, long l3) { + delegate.long256Column(name, l0, l1, l2, l3); + return this; + } + + @Override + public Sender longArray(@NotNull CharSequence name, long[] values) { + delegate.longArray(name, values); + return this; + } + + @Override + public Sender longArray(@NotNull CharSequence name, long[][] values) { + delegate.longArray(name, values); + return this; + } + + @Override + public Sender longArray(@NotNull CharSequence name, long[][][] values) { + delegate.longArray(name, values); + return this; + } + + @Override + public Sender longArray(@NotNull CharSequence name, LongArray values) { + delegate.longArray(name, values); + return this; + } + + @Override + public Sender longColumn(CharSequence name, long value) { + delegate.longColumn(name, value); + return this; + } + + @Override + public void reset() { + delegate.reset(); + } + + @Override + public Sender shortColumn(CharSequence name, short value) { + delegate.shortColumn(name, value); + return this; + } + + @Override + public Sender stringColumn(CharSequence name, CharSequence value) { + delegate.stringColumn(name, value); + return this; + } + + @Override + public Sender symbol(CharSequence name, CharSequence value) { + delegate.symbol(name, value); + return this; + } + + @Override + public Sender table(CharSequence table) { + delegate.table(table); + return this; + } + + @Override + public Sender timestampColumn(CharSequence name, long value, ChronoUnit unit) { + delegate.timestampColumn(name, value, unit); + return this; + } + + @Override + public Sender timestampColumn(CharSequence name, Instant value) { + delegate.timestampColumn(name, value); + return this; + } + + @Override + public Sender uuidColumn(CharSequence name, long lo, long hi) { + delegate.uuidColumn(name, lo, hi); + return this; + } + + long createdAtMillis() { + return createdAtMillis; + } + + Sender delegate() { + return delegate; + } + + long idleSinceMillis() { + return idleSinceMillis; + } + + boolean isInUse() { + return inUse; + } + + void markIdleAt(long nowMillis) { + idleSinceMillis = nowMillis; + } + + void markInUse() { + inUse = true; + } +} diff --git a/core/src/main/java/io/questdb/client/impl/QueryClientPool.java b/core/src/main/java/io/questdb/client/impl/QueryClientPool.java new file mode 100644 index 00000000..56d5ec7e --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/QueryClientPool.java @@ -0,0 +1,247 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.QueryException; +import io.questdb.client.cutlass.qwp.client.QwpQueryClient; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + +/** + * Elastic pool of {@link QueryWorker}s. Each worker pairs one + * {@link QwpQueryClient} (one WebSocket, one I/O thread) with one daemon + * dispatch thread. The pool keeps at least {@code minSize} workers warm and + * grows up to {@code maxSize} on demand; idle and over-age workers are + * reaped by the housekeeper. + *

+ * Worker creation involves a WebSocket upgrade (slow), so it happens + * outside the pool lock; an {@code inFlightCreations} counter keeps the + * cap check honest under concurrent acquires. + */ +public final class QueryClientPool { + + private final long acquireTimeoutMillis; + private final ArrayList all; + private final ArrayDeque available; + private final String configurationString; + private final long idleTimeoutMillis; + private final ReentrantLock lock = new ReentrantLock(); + private final long maxLifetimeMillis; + private final int maxSize; + private final int minSize; + private final AtomicInteger nextSlotIndex = new AtomicInteger(); + private final Condition workerReleased; + private volatile boolean closed; + private int inFlightCreations; + + QueryClientPool( + String configurationString, + int minSize, + int maxSize, + long acquireTimeoutMillis, + long idleTimeoutMillis, + long maxLifetimeMillis + ) { + if (minSize < 0 || maxSize < 1 || minSize > maxSize) { + throw new IllegalArgumentException("invalid pool sizing: min=" + minSize + ", max=" + maxSize); + } + this.configurationString = configurationString; + this.minSize = minSize; + this.maxSize = maxSize; + this.acquireTimeoutMillis = acquireTimeoutMillis; + this.idleTimeoutMillis = idleTimeoutMillis; + this.maxLifetimeMillis = maxLifetimeMillis; + this.all = new ArrayList<>(maxSize); + this.available = new ArrayDeque<>(maxSize); + this.workerReleased = lock.newCondition(); + int built = 0; + try { + for (int i = 0; i < minSize; i++) { + QueryWorker w = createUnlocked(); + w.start(); + all.add(w); + available.add(w); + built++; + } + } catch (RuntimeException e) { + for (int i = 0; i < built; i++) { + try { + all.get(i).shutdown(); + } catch (RuntimeException ignored) { + } + } + throw e; + } + } + + QueryWorker acquire() { + // Track remaining wait via awaitNanos's return value (canonical pattern): + // awaitNanos consumes from the budget on each wait and reports what is + // left; <= 0 means the budget is exhausted. + long remainingNanos = TimeUnit.MILLISECONDS.toNanos(acquireTimeoutMillis); + lock.lock(); + try { + while (true) { + if (closed) { + throw new QueryException((byte) 0, "QuestDB handle is closed"); + } + if (!available.isEmpty()) { + return available.pollFirst(); + } + if (all.size() + inFlightCreations < maxSize) { + inFlightCreations++; + lock.unlock(); + QueryWorker created; + try { + created = createUnlocked(); + created.start(); + } catch (RuntimeException e) { + lock.lock(); + inFlightCreations--; + workerReleased.signal(); + lock.unlock(); + throw new QueryException((byte) 0, + "failed to create query client: " + e.getMessage(), e); + } + lock.lock(); + inFlightCreations--; + if (closed) { + try { + created.shutdown(); + } catch (RuntimeException ignored) { + } + throw new QueryException((byte) 0, "QuestDB handle is closed"); + } + all.add(created); + return created; + } + if (remainingNanos <= 0) { + throw new QueryException((byte) 0, + "timed out waiting for a query client from the pool after " + + acquireTimeoutMillis + "ms"); + } + try { + remainingNanos = workerReleased.awaitNanos(remainingNanos); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new QueryException((byte) 0, + "interrupted while waiting for a query client from the pool"); + } + } + } finally { + if (lock.isHeldByCurrentThread()) { + lock.unlock(); + } + } + } + + public void close() { + ArrayList snapshot; + lock.lock(); + try { + if (closed) { + return; + } + closed = true; + workerReleased.signalAll(); + snapshot = new ArrayList<>(all); + } finally { + lock.unlock(); + } + // Cancel any in-flight queries first so execute() returns promptly, then + // join the worker threads and close their clients. Done outside the lock + // so a slow join doesn't keep the pool latched. + for (int i = 0; i < snapshot.size(); i++) { + snapshot.get(i).shutdown(); + } + } + + void reapIdle() { + if (closed) { + return; + } + long now = System.currentTimeMillis(); + ArrayList toShutdown = null; + lock.lock(); + try { + if (closed) { + return; + } + Iterator it = available.iterator(); + while (it.hasNext() && all.size() > minSize) { + QueryWorker w = it.next(); + boolean idleExpired = idleTimeoutMillis < Long.MAX_VALUE + && (now - w.idleSinceMillis()) >= idleTimeoutMillis; + boolean overAge = maxLifetimeMillis < Long.MAX_VALUE + && (now - w.createdAtMillis()) >= maxLifetimeMillis; + if (idleExpired || overAge) { + it.remove(); + all.remove(w); + if (toShutdown == null) { + toShutdown = new ArrayList<>(); + } + toShutdown.add(w); + } + } + } finally { + lock.unlock(); + } + if (toShutdown != null) { + for (int i = 0, n = toShutdown.size(); i < n; i++) { + try { + toShutdown.get(i).shutdown(); + } catch (RuntimeException ignored) { + } + } + } + } + + void release(QueryWorker w) { + long now = System.currentTimeMillis(); + w.markIdleAt(now); + lock.lock(); + try { + if (closed) { + return; + } + available.addLast(w); + workerReleased.signal(); + } finally { + lock.unlock(); + } + } + + private QueryWorker createUnlocked() { + QwpQueryClient client = QwpQueryClient.fromConfig(configurationString); + client.connect(); + return new QueryWorker(client, this, nextSlotIndex.getAndIncrement()); + } +} diff --git a/core/src/main/java/io/questdb/client/impl/QueryImpl.java b/core/src/main/java/io/questdb/client/impl/QueryImpl.java new file mode 100644 index 00000000..6d493279 --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/QueryImpl.java @@ -0,0 +1,295 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.Completion; +import io.questdb.client.Query; +import io.questdb.client.QueryException; +import io.questdb.client.cutlass.qwp.client.QwpBindSetter; +import io.questdb.client.cutlass.qwp.client.QwpBindValues; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatch; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler; +import io.questdb.client.cutlass.qwp.client.QwpQueryClient; +import io.questdb.client.cutlass.qwp.client.QwpServerInfo; +import io.questdb.client.std.str.StringSink; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + +/** + * Per-thread implementation of {@link Query}. Holds the configured query + * state (SQL, optional binds, handler), an inner {@link Completion}, and a + * wrapping {@link QwpColumnBatchHandler} that forwards callbacks to the user + * handler and signals the Completion on terminal events. + *

+ * Lifecycle: {@link QuestDBImpl#query()} returns a per-thread instance, reset + * to empty if it was in a terminal state. {@link #submit()} acquires a + * worker, dispatches, and returns the cached {@link Completion}. + */ +final class QueryImpl implements Query { + + private final InnerCompletion completion = new InnerCompletion(); + private final Condition doneCondition; + private final ReentrantLock doneLock = new ReentrantLock(); + private final QueryClientPool pool; + private final QwpBindSetter wireBinds = this::applyBinds; + private final WrappingHandler wrappingHandler = new WrappingHandler(); + private final StringSink sqlBuffer = new StringSink(); + private QwpBindSetter userBinds; + private QwpColumnBatchHandler userHandler; + private volatile QueryWorker currentWorker; + private volatile boolean done = true; + private volatile String resultMessage; + private volatile byte resultStatus; + private volatile Throwable unexpectedError; + + QueryImpl(QueryClientPool pool) { + this.pool = pool; + this.doneCondition = doneLock.newCondition(); + } + + @Override + public void abandon() { + if (!done) { + throw new IllegalStateException("a previous submit() is still in flight; await the Completion first"); + } + userBinds = null; + userHandler = null; + sqlBuffer.clear(); + } + + @Override + public Query binds(QwpBindSetter binds) { + this.userBinds = binds; + return this; + } + + @Override + public Query handler(QwpColumnBatchHandler handler) { + this.userHandler = handler; + return this; + } + + @Override + public Query sql(CharSequence sql) { + sqlBuffer.clear(); + sqlBuffer.put(sql); + return this; + } + + @Override + public Completion submit() { + if (sqlBuffer.length() == 0) { + throw new IllegalStateException("sql is required"); + } + if (userHandler == null) { + throw new IllegalStateException("handler is required"); + } + if (!done) { + throw new IllegalStateException("a previous submit() is still in flight; await the Completion first"); + } + QueryWorker w = pool.acquire(); + // Reset terminal state under the lock so a stale signal from a prior + // run can't be observed by the upcoming await(). + doneLock.lock(); + try { + done = false; + resultStatus = 0; + resultMessage = null; + unexpectedError = null; + currentWorker = w; + } finally { + doneLock.unlock(); + } + w.dispatch(this); + return completion; + } + + void runOn(QwpQueryClient client) { + client.execute(sqlBuffer.toString(), wireBinds, wrappingHandler); + } + + /** + * Signals an unexpected error from the worker thread (for example, an + * exception escaping {@code execute()} before any handler callback). + */ + void signalUnexpected(Throwable t) { + signalDone((byte) 0, t.getMessage() != null ? t.getMessage() : t.getClass().getSimpleName(), t); + } + + private void applyBinds(QwpBindValues binds) { + QwpBindSetter setter = userBinds; + if (setter != null) { + setter.apply(binds); + } + } + + private void signalDone(byte status, String message, Throwable unexpected) { + doneLock.lock(); + try { + if (done) { + return; + } + this.resultStatus = status; + this.resultMessage = message; + this.unexpectedError = unexpected; + this.done = true; + this.currentWorker = null; + doneCondition.signalAll(); + } finally { + doneLock.unlock(); + } + } + + private final class InnerCompletion implements Completion { + + @Override + public void await() throws InterruptedException { + doneLock.lock(); + try { + while (!done) { + doneCondition.await(); + } + } finally { + doneLock.unlock(); + } + throwIfFailed(); + } + + @Override + public boolean await(long timeout, TimeUnit unit) throws InterruptedException { + long remaining = unit.toNanos(timeout); + doneLock.lock(); + try { + while (!done) { + if (remaining <= 0) { + return false; + } + remaining = doneCondition.awaitNanos(remaining); + } + } finally { + doneLock.unlock(); + } + throwIfFailed(); + return true; + } + + @Override + public void cancel() { + QueryWorker w = currentWorker; + if (w != null && !done) { + w.cancelInFlight(); + } + } + + @Override + public boolean isDone() { + return done; + } + + private void throwIfFailed() { + Throwable unexpected = unexpectedError; + if (unexpected != null) { + throw new QueryException(resultStatus, resultMessage, unexpected); + } + if (resultStatus != 0) { + throw new QueryException(resultStatus, resultMessage); + } + } + } + + private final class WrappingHandler implements QwpColumnBatchHandler { + + @Override + public void onBatch(QwpColumnBatch batch) { + userHandler.onBatch(batch); + } + + @Override + public void onEnd(long totalRows) { + try { + userHandler.onEnd(totalRows); + } finally { + signalDone((byte) 0, null, null); + } + } + + @Override + public void onEnd(long requestId, long totalRows) { + try { + userHandler.onEnd(requestId, totalRows); + } finally { + signalDone((byte) 0, null, null); + } + } + + @Override + public void onError(byte status, String message) { + try { + userHandler.onError(status, message); + } finally { + signalDone(status, message, null); + } + } + + @Override + public void onError(long requestId, byte status, String message) { + try { + userHandler.onError(requestId, status, message); + } finally { + signalDone(status, message, null); + } + } + + @Override + public void onExecDone(short opType, long rowsAffected) { + try { + userHandler.onExecDone(opType, rowsAffected); + } finally { + signalDone((byte) 0, null, null); + } + } + + @Override + public void onExecDone(long requestId, short opType, long rowsAffected) { + try { + userHandler.onExecDone(requestId, opType, rowsAffected); + } finally { + signalDone((byte) 0, null, null); + } + } + + @Override + public void onFailoverReset(QwpServerInfo newNode) { + userHandler.onFailoverReset(newNode); + } + + @Override + public void onFailoverReset(long requestId, QwpServerInfo newNode) { + userHandler.onFailoverReset(requestId, newNode); + } + } +} diff --git a/core/src/main/java/io/questdb/client/impl/QueryWorker.java b/core/src/main/java/io/questdb/client/impl/QueryWorker.java new file mode 100644 index 00000000..b8e2ccd6 --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/QueryWorker.java @@ -0,0 +1,168 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.cutlass.qwp.client.QwpQueryClient; + +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + +/** + * Pairs one {@link QwpQueryClient} with one dedicated thread. The worker thread + * loops, waiting for {@link #dispatch} to hand it a {@link QueryImpl}, then + * runs {@code execute()} synchronously and releases itself back to the pool + * when the call returns. + *

+ * The pooled query client's own I/O thread continues to drive the wire; the + * worker thread exists only to keep {@code execute()} off the application's + * submitting thread. Handler callbacks ({@code onBatch}, {@code onEnd}, + * {@code onError}) still run on the client's I/O thread. + */ +public final class QueryWorker { + + static final long SHUTDOWN_JOIN_MILLIS = 5_000; + private final QwpQueryClient client; + private final long createdAtMillis; + private final QueryClientPool pool; + private final Condition signalCondition; + private final ReentrantLock signalLock = new ReentrantLock(); + private final Thread thread; + private volatile QueryImpl current; + private volatile long idleSinceMillis; + private volatile boolean shuttingDown; + + public QueryWorker(QwpQueryClient client, QueryClientPool pool, int slotIndex) { + this.client = client; + this.pool = pool; + this.signalCondition = signalLock.newCondition(); + this.thread = new Thread(this::runLoop, "questdb-query-worker-" + slotIndex); + this.thread.setDaemon(true); + this.createdAtMillis = System.currentTimeMillis(); + this.idleSinceMillis = this.createdAtMillis; + } + + long createdAtMillis() { + return createdAtMillis; + } + + long idleSinceMillis() { + return idleSinceMillis; + } + + void markIdleAt(long nowMillis) { + idleSinceMillis = nowMillis; + } + + /** + * Cancels the in-flight query on this worker's client. Safe to call from + * any thread; harmless if the worker is idle. + */ + void cancelInFlight() { + try { + client.cancel(); + } catch (RuntimeException ignored) { + // cancel() is best-effort; an already-completed query is fine. + } + } + + /** + * Returns the {@link QwpQueryClient} this worker drives. Exposed for + * introspection and tests; callers must not invoke {@code execute()} on + * it directly because that would race the worker's own dispatch loop. + */ + public QwpQueryClient client() { + return client; + } + + void shutdown() { + shuttingDown = true; + signalLock.lock(); + try { + signalCondition.signalAll(); + } finally { + signalLock.unlock(); + } + // If a query is in flight on this worker, ask the client to abort so + // execute() returns promptly and the thread can exit before join times + // out. cancel() is documented as thread-safe and is a no-op when idle. + try { + client.cancel(); + } catch (RuntimeException ignored) { + } + try { + thread.join(SHUTDOWN_JOIN_MILLIS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + try { + client.close(); + } catch (RuntimeException ignored) { + } + } + + void start() { + thread.start(); + } + + /** + * Hands a configured {@link QueryImpl} to this worker. The caller must + * have just acquired this worker via QueryClientPool#acquire(long). + */ + void dispatch(QueryImpl q) { + signalLock.lock(); + try { + current = q; + signalCondition.signal(); + } finally { + signalLock.unlock(); + } + } + + private void runLoop() { + while (!shuttingDown) { + QueryImpl q; + signalLock.lock(); + try { + while (current == null && !shuttingDown) { + signalCondition.awaitUninterruptibly(); + } + if (shuttingDown) { + return; + } + q = current; + } finally { + signalLock.unlock(); + } + try { + q.runOn(client); + } catch (Throwable t) { + q.signalUnexpected(t); + } finally { + current = null; + pool.release(this); + } + } + } +} diff --git a/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java b/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java new file mode 100644 index 00000000..672bd33b --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java @@ -0,0 +1,129 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.Completion; +import io.questdb.client.QuestDB; +import io.questdb.client.Query; +import io.questdb.client.Sender; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler; + +/** + * Implementation of {@link QuestDB}. Owns the elastic {@link SenderPool} + * and {@link QueryClientPool}, a {@link PoolHousekeeper} that reaps idle + * slots, and a {@link ThreadLocal} of {@link QueryImpl} instances so that + * {@link #query()} is allocation-free after the first call on each thread. + */ +public final class QuestDBImpl implements QuestDB { + + private final PoolHousekeeper housekeeper; + private final QueryClientPool queryPool; + private final ThreadLocal queryThreadLocal; + private final SenderPool senderPool; + private volatile boolean closed; + + public QuestDBImpl( + String ingestConfig, + String queryConfig, + int senderMin, + int senderMax, + int queryMin, + int queryMax, + long acquireTimeoutMillis, + long idleTimeoutMillis, + long maxLifetimeMillis, + long housekeeperIntervalMillis + ) { + SenderPool builtSenderPool = null; + QueryClientPool builtQueryPool = null; + PoolHousekeeper builtHousekeeper = null; + try { + builtSenderPool = new SenderPool( + ingestConfig, senderMin, senderMax, acquireTimeoutMillis, + idleTimeoutMillis, maxLifetimeMillis); + builtQueryPool = new QueryClientPool( + queryConfig, queryMin, queryMax, acquireTimeoutMillis, + idleTimeoutMillis, maxLifetimeMillis); + builtHousekeeper = new PoolHousekeeper(builtSenderPool, builtQueryPool, housekeeperIntervalMillis); + builtHousekeeper.start(); + } catch (RuntimeException e) { + if (builtHousekeeper != null) { + builtHousekeeper.stop(); + } + if (builtQueryPool != null) { + builtQueryPool.close(); + } + if (builtSenderPool != null) { + builtSenderPool.close(); + } + throw e; + } + this.senderPool = builtSenderPool; + this.queryPool = builtQueryPool; + this.housekeeper = builtHousekeeper; + this.queryThreadLocal = ThreadLocal.withInitial(() -> new QueryImpl(queryPool)); + } + + @Override + public Sender borrowSender() { + return senderPool.borrow(); + } + + @Override + public void close() { + if (closed) { + return; + } + closed = true; + housekeeper.stop(); + queryPool.close(); + senderPool.close(); + } + + @Override + public Completion executeSql(CharSequence sql, QwpColumnBatchHandler handler) { + return query().sql(sql).handler(handler).submit(); + } + + @Override + public Query newQuery() { + return new QueryImpl(queryPool); + } + + @Override + public Query query() { + return queryThreadLocal.get(); + } + + @Override + public void releaseSender() { + senderPool.releaseCurrentThread(); + } + + @Override + public Sender sender() { + return senderPool.pinToCurrentThread(); + } +} diff --git a/core/src/main/java/io/questdb/client/impl/SenderPool.java b/core/src/main/java/io/questdb/client/impl/SenderPool.java new file mode 100644 index 00000000..8eaae97a --- /dev/null +++ b/core/src/main/java/io/questdb/client/impl/SenderPool.java @@ -0,0 +1,301 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.impl; + +import io.questdb.client.Sender; +import io.questdb.client.cutlass.line.LineSenderException; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + +/** + * Elastic pool of {@link Sender} instances, each wrapped in a + * {@link PooledSender} decorator. The pool keeps at least {@code minSize} + * connections warm, grows on demand up to {@code maxSize}, and lets the + * housekeeper reap slots that have idled longer than {@code idleTimeoutMillis} + * or aged past {@code maxLifetimeMillis} (with {@code minSize} respected at + * all times). + *

+ * The hot borrow / return path takes a {@link ReentrantLock} but does no + * per-call allocation; the underlying {@link ArrayDeque} of free decorators + * is pre-sized to {@code maxSize}. + *

+ * Connection creation happens outside the lock so a slow connect (TLS + * handshake, DNS) does not block other borrowers or the housekeeper. The + * pool tracks in-flight creations via {@code inFlightCreations} so the cap + * check ({@code allSize + inFlightCreations < maxSize}) stays correct under + * concurrent borrows. + */ +public final class SenderPool implements AutoCloseable { + + private final long acquireTimeoutMillis; + private final ArrayList all; + private final ArrayDeque available; + private final String configurationString; + private final long idleTimeoutMillis; + private final ReentrantLock lock = new ReentrantLock(); + private final long maxLifetimeMillis; + private final int maxSize; + private final int minSize; + private final Condition slotReleased; + private final ThreadLocal threadAffine = new ThreadLocal<>(); + private volatile boolean closed; + private int inFlightCreations; + + public SenderPool( + String configurationString, + int minSize, + int maxSize, + long acquireTimeoutMillis, + long idleTimeoutMillis, + long maxLifetimeMillis + ) { + if (minSize < 0 || maxSize < 1 || minSize > maxSize) { + throw new IllegalArgumentException("invalid pool sizing: min=" + minSize + ", max=" + maxSize); + } + this.configurationString = configurationString; + this.minSize = minSize; + this.maxSize = maxSize; + this.acquireTimeoutMillis = acquireTimeoutMillis; + this.idleTimeoutMillis = idleTimeoutMillis; + this.maxLifetimeMillis = maxLifetimeMillis; + this.all = new ArrayList<>(maxSize); + this.available = new ArrayDeque<>(maxSize); + this.slotReleased = lock.newCondition(); + // Pre-warm minSize connections. + int built = 0; + try { + for (int i = 0; i < minSize; i++) { + PooledSender ps = createUnlocked(); + all.add(ps); + available.add(ps); + built++; + } + } catch (RuntimeException e) { + for (int i = 0; i < built; i++) { + try { + all.get(i).delegate().close(); + } catch (RuntimeException ignored) { + } + } + throw e; + } + } + + public PooledSender borrow() { + // Track remaining wait via awaitNanos's return value (canonical pattern): + // awaitNanos consumes from the budget on each wait and reports what is + // left; <= 0 means the budget is exhausted. + long remainingNanos = TimeUnit.MILLISECONDS.toNanos(acquireTimeoutMillis); + lock.lock(); + try { + while (true) { + if (closed) { + throw new LineSenderException("QuestDB handle is closed"); + } + if (!available.isEmpty()) { + PooledSender s = available.pollFirst(); + s.markInUse(); + return s; + } + if (all.size() + inFlightCreations < maxSize) { + inFlightCreations++; + lock.unlock(); + PooledSender created; + try { + created = createUnlocked(); + } catch (RuntimeException e) { + lock.lock(); + inFlightCreations--; + slotReleased.signal(); + lock.unlock(); + throw e; + } + lock.lock(); + inFlightCreations--; + if (closed) { + // Pool was closed mid-creation -- destroy the new connection + // rather than leaking it. Other waiters have been signaled + // by close() already. + try { + created.delegate().close(); + } catch (RuntimeException ignored) { + } + throw new LineSenderException("QuestDB handle is closed"); + } + all.add(created); + created.markInUse(); + return created; + } + if (remainingNanos <= 0) { + throw new LineSenderException( + "timed out waiting for a Sender from the pool after " + acquireTimeoutMillis + "ms"); + } + try { + remainingNanos = slotReleased.awaitNanos(remainingNanos); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new LineSenderException("interrupted while waiting for a Sender from the pool"); + } + } + } finally { + if (lock.isHeldByCurrentThread()) { + lock.unlock(); + } + } + } + + @Override + public void close() { + lock.lock(); + try { + if (closed) { + return; + } + closed = true; + threadAffine.remove(); + slotReleased.signalAll(); + } finally { + lock.unlock(); + } + // Snapshot of underlying Senders to close, taken outside the lock so + // a slow real-close() doesn't keep the pool latched. + for (int i = 0; i < all.size(); i++) { + try { + all.get(i).delegate().close(); + } catch (RuntimeException ignored) { + } + } + } + + public void giveBack(PooledSender s) { + long now = System.currentTimeMillis(); + s.markIdleAt(now); + lock.lock(); + try { + if (closed) { + // Pool already shut down: don't requeue; let close() finish destroying. + return; + } + available.addLast(s); + slotReleased.signal(); + } finally { + lock.unlock(); + } + } + + public PooledSender pinToCurrentThread() { + PooledSender pinned = threadAffine.get(); + if (pinned != null) { + return pinned; + } + PooledSender s = borrow(); + threadAffine.set(s); + return s; + } + + /** + * Closes idle slots that have exceeded {@code idleTimeoutMillis} or that + * have aged past {@code maxLifetimeMillis}. Never shrinks below + * {@code minSize}. Called by the {@link PoolHousekeeper} on its tick. + */ + public void reapIdle() { + if (closed) { + return; + } + long now = System.currentTimeMillis(); + ArrayList toClose = null; + lock.lock(); + try { + if (closed) { + return; + } + Iterator it = available.iterator(); + while (it.hasNext() && all.size() > minSize) { + PooledSender s = it.next(); + boolean idleExpired = idleTimeoutMillis < Long.MAX_VALUE + && (now - s.idleSinceMillis()) >= idleTimeoutMillis; + boolean overAge = maxLifetimeMillis < Long.MAX_VALUE + && (now - s.createdAtMillis()) >= maxLifetimeMillis; + if (idleExpired || overAge) { + it.remove(); + all.remove(s); + if (toClose == null) { + toClose = new ArrayList<>(); + } + toClose.add(s); + } + } + } finally { + lock.unlock(); + } + if (toClose != null) { + for (int i = 0, n = toClose.size(); i < n; i++) { + try { + toClose.get(i).delegate().close(); + } catch (RuntimeException ignored) { + } + } + } + } + + /** Snapshot of the number of idle slots. For tests and introspection. */ + public int availableSize() { + lock.lock(); + try { + return available.size(); + } finally { + lock.unlock(); + } + } + + /** Snapshot of the total number of live slots (idle + in-use). For tests and introspection. */ + public int totalSize() { + lock.lock(); + try { + return all.size(); + } finally { + lock.unlock(); + } + } + + public void releaseCurrentThread() { + PooledSender pinned = threadAffine.get(); + if (pinned == null) { + return; + } + threadAffine.remove(); + pinned.close(); + } + + private PooledSender createUnlocked() { + Sender raw = Sender.fromConfig(configurationString); + return new PooledSender(raw, this); + } +} diff --git a/core/src/test/java/io/questdb/client/test/QuestDBBuilderTest.java b/core/src/test/java/io/questdb/client/test/QuestDBBuilderTest.java new file mode 100644 index 00000000..69cb4645 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/QuestDBBuilderTest.java @@ -0,0 +1,135 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test; + +import io.questdb.client.QuestDB; +import org.junit.Assert; +import org.junit.Test; + +public class QuestDBBuilderTest { + + @Test + public void testMissingIngestConfigThrows() { + try { + QuestDB.builder().queryConfig("ws::addr=h:9000;").build().close(); + Assert.fail(); + } catch (IllegalStateException e) { + Assert.assertTrue(e.getMessage().contains("ingest")); + } + } + + @Test + public void testMissingQueryConfigThrows() { + try { + QuestDB.builder().ingestConfig("http::addr=h:9000;").build().close(); + Assert.fail(); + } catch (IllegalStateException e) { + Assert.assertTrue(e.getMessage().contains("query")); + } + } + + @Test + public void testNegativeAcquireTimeoutRejected() { + try { + QuestDB.builder().acquireTimeoutMillis(-1); + Assert.fail(); + } catch (IllegalArgumentException ignored) { + } + } + + @Test + public void testNegativePoolSizesRejected() { + try { + QuestDB.builder().senderPoolSize(0); + Assert.fail(); + } catch (IllegalArgumentException ignored) { + } + try { + QuestDB.builder().queryPoolSize(0); + Assert.fail(); + } catch (IllegalArgumentException ignored) { + } + } + + @Test + public void testBuilderCallAfterFromConfigOverridesPoolKeysFromString() { + // Build to a dead address with a forced exhaustion timeout so we can read + // the timeout off the resulting LineSenderException. fromConfig() sets + // acquire_timeout_ms=10000; subsequent acquireTimeoutMillis(150) wins + // because the builder applies last-write-wins. + try (io.questdb.client.QuestDB ignored = QuestDB.builder() + .fromConfig("http::addr=127.0.0.1:1;protocol_version=2;auto_flush=off;" + + "sender_pool_min=1;sender_pool_max=1;query_pool_min=1;query_pool_max=1;" + + "acquire_timeout_ms=10000;idle_timeout_ms=0;max_lifetime_ms=0;") + .queryConfig("ws::addr=127.0.0.1:1;auth_timeout_ms=50;failover=off;query_pool_min=0;query_pool_max=0;") + .acquireTimeoutMillis(150) + .build()) { + Assert.fail("expected build to fail (no live server)"); + } catch (RuntimeException expected) { + // Either sender or query pool build fails -- both are fine, both prove the + // builder is wired through. The pool-config keys in the strings did not + // crash the parsers (test would have thrown InvalidArgument earlier). + } + } + + @Test + public void testConnectStringWithPoolKeysAppliedToBuilder() { + // Build will fail (dead address) but we can verify the timeout came from + // the connect string by measuring how long borrowSender blocks would take. + // Easier: just assert the build path doesn't choke on the pool keys. + try (io.questdb.client.QuestDB ignored = QuestDB.builder() + .ingestConfig("http::addr=127.0.0.1:1;protocol_version=2;auto_flush=off;") + .queryConfig("ws::addr=127.0.0.1:1;auth_timeout_ms=100;failover=off;") + .senderPoolSize(1) + .queryPoolSize(1) + .acquireTimeoutMillis(100) + .build()) { + Assert.fail("build should fail with dead query address"); + } catch (RuntimeException expected) { + // Validated by absence of an IllegalArgumentException for pool keys. + } + } + + @Test + public void testQueryPoolBuildFailureUnwindsSenderPool() { + // Sender pool builds fine (http connects lazily); query pool fails because + // ws::127.0.0.1:1 is not a live QuestDB. The handle must clean up the + // already-built sender pool rather than leaking N Senders. + try { + QuestDB.builder() + .ingestConfig("http::addr=127.0.0.1:1;protocol_version=2;auto_flush=off;") + .queryConfig("ws::addr=127.0.0.1:1;auth_timeout_ms=200;failover=off;") + .senderPoolSize(2) + .queryPoolSize(2) + .acquireTimeoutMillis(500) + .build() + .close(); + Assert.fail("expected build to fail when query pool cannot connect"); + } catch (RuntimeException expected) { + // The exact exception type comes from QwpQueryClient.connect(); + // we only assert the build failed so we know cleanup ran. + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/example/QuestDBExamples.java b/core/src/test/java/io/questdb/client/test/example/QuestDBExamples.java new file mode 100644 index 00000000..3c4b6374 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/example/QuestDBExamples.java @@ -0,0 +1,199 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.example; + +import io.questdb.client.Completion; +import io.questdb.client.QuestDB; +import io.questdb.client.Query; +import io.questdb.client.Sender; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatch; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler; + +import java.util.concurrent.TimeUnit; + +/** + * Examples for the {@link QuestDB} facade -- the high-level handle that + * pools both Senders (ingest) and query clients (egress). + *

+ * Create one {@code QuestDB} per deployment, share it across threads, and + * close it at shutdown. Borrows and releases are zero-allocation at steady + * state; the per-thread {@link Query} handle is cached in a ThreadLocal. + */ +public class QuestDBExamples { + + public static void main(String[] args) throws Exception { + // 1. Connect with a single configuration string. The same server list + // serves both ingest (HTTP) and egress (WebSocket on the same port); + // QuestDB derives the egress URL automatically. + try (QuestDB db = QuestDB.connect("http::addr=localhost:9000;")) { + ingestWithBorrowedSender(db); + ingestWithThreadAffineSender(db); + queryOneShot(db); + queryWithBinds(db); + cancelExample(db); + } + + // 2. Authenticated connect: token auth is translated to a Bearer + // Authorization header on the egress side. + try (QuestDB db = QuestDB.connect( + "http::addr=db.questdb.cloud:9000;token=YOUR_TOKEN_HERE;")) { + // ... use db ... + db.executeSql("SELECT 1", new PrintingHandler()).await(); + } + + // 3. Custom pool sizing and timeouts via the builder. Use this when + // ingest and egress configs differ (different transports, separate + // address lists), or when you need to override defaults. + try (QuestDB db = QuestDB.builder() + .ingestConfig("http::addr=ingest.cluster:9000;") + .queryConfig("ws::addr=read-replica.cluster:9000;") + .senderPoolSize(8) + .queryPoolSize(4) + .acquireTimeoutMillis(10_000) + .build()) { + // ... use db ... + db.executeSql("SELECT 1", new PrintingHandler()).await(); + } + } + + /** + * Cancel mid-query: the handler observes {@code onError} with the cancel + * status, and {@code await()} throws {@code QueryException} with that + * status. If the query completed before cancel landed, {@code await()} + * returns normally; either way the Completion reaches a terminal state. + */ + static void cancelExample(QuestDB db) { + Completion c = db.executeSql( + "SELECT * FROM big_table ORDER BY ts", + new PrintingHandler()); + // ... some condition decides to abort ... + c.cancel(); + try { + c.await(); + } catch (Exception cancelled) { + // expected when cancel won the race + } + } + + /** + * Borrowed Sender: leases one from the pool, flushes pending rows on + * close(), returns to the pool. Use this for short-lived or event-loop + * callers where pinning a Sender to a thread is not appropriate. + */ + static void ingestWithBorrowedSender(QuestDB db) { + try (Sender s = db.borrowSender()) { + s.table("trades") + .symbol("symbol", "BTC-USD") + .doubleColumn("price", 42_500.50) + .longColumn("size", 100) + .atNow(); + // close() flushes -- no need to call flush() yourself. + } + } + + /** + * Thread-affine Sender: the first call on a thread leases one and pins it; + * subsequent calls on the same thread return the same instance with zero + * borrow overhead. Best for long-lived dedicated producer threads. + *

+ * Call {@link QuestDB#releaseSender()} on threads borrowed from pools you + * don't own (Netty event loops, etc.) before they're recycled. + */ + static void ingestWithThreadAffineSender(QuestDB db) { + Sender s = db.sender(); + for (int i = 0; i < 1_000; i++) { + s.table("trades") + .symbol("symbol", "BTC-USD") + .doubleColumn("price", 42_500.50 + i) + .longColumn("size", 100) + .atNow(); + } + s.flush(); + // Not strictly required: db.close() reaps pinned Senders. Call it + // only when handing this thread back to a foreign pool. + // db.releaseSender(); + } + + /** + * One-shot query, no bind parameters. {@link QuestDB#executeSql} returns + * a {@link Completion} that you can {@code await()} synchronously, time + * out on, or cancel. + */ + static void queryOneShot(QuestDB db) throws InterruptedException { + Completion c = db.executeSql( + "SELECT price FROM trades WHERE symbol = 'BTC-USD' LIMIT 10", + new PrintingHandler()); + c.await(); + } + + /** + * Query with bind parameters. Use {@link QuestDB#query()} to get the + * per-thread Query builder, then set SQL, binds (via QwpBindSetter), and + * handler. + *

+ * The same SQL text reuses the server's compiled-factory cache -- bind + * values supply the per-call inputs. Interpolating values into the SQL + * string defeats that cache. + */ + static void queryWithBinds(QuestDB db) throws InterruptedException { + Query q = db.query() + .sql("SELECT price FROM trades WHERE symbol = $1 LIMIT $2") + .binds(binds -> { + binds.setVarchar(0, "BTC-USD"); + binds.setLong(1, 10L); + }) + .handler(new PrintingHandler()); + Completion c = q.submit(); + // Optional timeout: returns false if the query is still in flight. + if (!c.await(5, TimeUnit.SECONDS)) { + c.cancel(); + c.await(); + } + } + + /** + * Minimal handler: prints each row's first column. Real applications + * implement a stateful handler that aggregates batches; the same + * instance can be reused across submits for zero allocation. + */ + private static final class PrintingHandler implements QwpColumnBatchHandler { + @Override + public void onBatch(QwpColumnBatch batch) { + for (int r = 0; r < batch.getRowCount(); r++) { + System.out.println(batch.getDoubleValue(0, r)); + } + } + + @Override + public void onEnd(long totalRows) { + System.out.println("done: " + totalRows + " rows"); + } + + @Override + public void onError(byte status, String message) { + System.err.println("egress error: status=" + status + ", message=" + message); + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/impl/ConfigStringTranslatorTest.java b/core/src/test/java/io/questdb/client/test/impl/ConfigStringTranslatorTest.java new file mode 100644 index 00000000..7fdb6e4b --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/impl/ConfigStringTranslatorTest.java @@ -0,0 +1,159 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.impl; + +import io.questdb.client.impl.ConfigStringTranslator; +import org.junit.Assert; +import org.junit.Test; + +public class ConfigStringTranslatorTest { + + @Test + public void testEmptyConfigIsRejected() { + try { + ConfigStringTranslator.deriveBothSides(""); + Assert.fail(); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("empty")); + } + } + + @Test + public void testHttpInputPassesThroughAndDerivesWs() { + ConfigStringTranslator.Bundle bundle = ConfigStringTranslator.deriveBothSides( + "http::addr=db.host:9000;token=secret;"); + Assert.assertEquals("http::addr=db.host:9000;token=secret;", bundle.ingestConfig); + Assert.assertEquals("ws::addr=db.host:9000;auth=Bearer secret;", bundle.queryConfig); + // No pool keys -> all defaults preserved. + Assert.assertEquals(ConfigStringTranslator.PoolConfig.UNSET, bundle.poolConfig.senderPoolMin); + Assert.assertEquals(ConfigStringTranslator.PoolConfig.UNSET, bundle.poolConfig.acquireTimeoutMillis); + } + + @Test + public void testHttpsInputDerivesWss() { + ConfigStringTranslator.Bundle bundle = ConfigStringTranslator.deriveBothSides( + "https::addr=db.host:9000;tls_verify=on;"); + Assert.assertEquals("https::addr=db.host:9000;tls_verify=on;", bundle.ingestConfig); + Assert.assertEquals("wss::addr=db.host:9000;tls_verify=on;", bundle.queryConfig); + } + + @Test + public void testInvalidPoolValueIsRejected() { + try { + ConfigStringTranslator.deriveBothSides("http::addr=h:9000;sender_pool_max=notanumber;"); + Assert.fail(); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("sender_pool_max")); + } + } + + @Test + public void testMissingAddrIsRejected() { + try { + ConfigStringTranslator.deriveBothSides("http::token=x;"); + Assert.fail(); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("addr")); + } + } + + @Test + public void testPoolKeysAreExtractedAndStripped() { + ConfigStringTranslator.Bundle bundle = ConfigStringTranslator.deriveBothSides( + "http::addr=db.host:9000;sender_pool_min=2;sender_pool_max=16;" + + "query_pool_min=1;query_pool_max=4;acquire_timeout_ms=10000;" + + "idle_timeout_ms=30000;max_lifetime_ms=600000;housekeeper_interval_ms=2000;"); + + // Pool keys must be stripped from both config strings so the downstream + // Sender / QwpQueryClient parsers never see them. + Assert.assertFalse(bundle.ingestConfig.contains("sender_pool")); + Assert.assertFalse(bundle.ingestConfig.contains("query_pool")); + Assert.assertFalse(bundle.ingestConfig.contains("timeout_ms")); + Assert.assertFalse(bundle.queryConfig.contains("sender_pool")); + Assert.assertFalse(bundle.queryConfig.contains("query_pool")); + Assert.assertFalse(bundle.queryConfig.contains("timeout_ms")); + + // addr must survive on both sides. + Assert.assertTrue(bundle.ingestConfig.contains("addr=db.host:9000")); + Assert.assertTrue(bundle.queryConfig.contains("addr=db.host:9000")); + + // Pool values must surface on the PoolConfig. + Assert.assertEquals(2, bundle.poolConfig.senderPoolMin); + Assert.assertEquals(16, bundle.poolConfig.senderPoolMax); + Assert.assertEquals(1, bundle.poolConfig.queryPoolMin); + Assert.assertEquals(4, bundle.poolConfig.queryPoolMax); + Assert.assertEquals(10_000L, bundle.poolConfig.acquireTimeoutMillis); + Assert.assertEquals(30_000L, bundle.poolConfig.idleTimeoutMillis); + Assert.assertEquals(600_000L, bundle.poolConfig.maxLifetimeMillis); + Assert.assertEquals(2_000L, bundle.poolConfig.housekeeperIntervalMillis); + } + + @Test + public void testPoolKeysInterleavedWithRegularKeys() { + // Pool keys at arbitrary positions must still be stripped and the + // surviving keys must remain in the original order. + ConfigStringTranslator.Bundle bundle = ConfigStringTranslator.deriveBothSides( + "http::sender_pool_max=8;addr=h:9000;query_pool_max=2;token=t;idle_timeout_ms=5000;"); + Assert.assertTrue(bundle.ingestConfig.contains("addr=h:9000")); + Assert.assertTrue(bundle.ingestConfig.contains("token=t")); + Assert.assertFalse(bundle.ingestConfig.contains("pool")); + Assert.assertFalse(bundle.ingestConfig.contains("idle")); + Assert.assertEquals(8, bundle.poolConfig.senderPoolMax); + Assert.assertEquals(2, bundle.poolConfig.queryPoolMax); + Assert.assertEquals(5_000L, bundle.poolConfig.idleTimeoutMillis); + } + + @Test + public void testTcpSchemaIsRejected() { + try { + ConfigStringTranslator.deriveBothSides("tcp::addr=h:9009;"); + Assert.fail(); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("supports schemas")); + } + } + + @Test + public void testUsernamePasswordRejectedForWsDerivation() { + try { + ConfigStringTranslator.deriveBothSides( + "http::addr=h:9000;username=u;password=p;"); + Assert.fail(); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("username/password")); + } + } + + @Test + public void testWsInputPassesThroughAndDerivesHttp() { + ConfigStringTranslator.Bundle bundle = ConfigStringTranslator.deriveBothSides( + "ws::addr=db.host:9000;auth=Bearer foo;"); + Assert.assertEquals("ws::addr=db.host:9000;auth=Bearer foo;", bundle.queryConfig); + Assert.assertTrue( + "expected ingest config to start with http::; got: " + bundle.ingestConfig, + bundle.ingestConfig.startsWith("http::")); + Assert.assertTrue(bundle.ingestConfig.contains("addr=db.host:9000")); + } +} diff --git a/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java b/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java new file mode 100644 index 00000000..01ce0986 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java @@ -0,0 +1,49 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.impl; + +import io.questdb.client.cutlass.qwp.client.QwpQueryClient; +import io.questdb.client.impl.QueryWorker; +import org.junit.Assert; +import org.junit.Test; + +public class QueryWorkerTest { + + /** + * Exercises {@link QueryWorker#client()} -- a pure getter exposed for + * introspection. The worker is constructed but never started, so no + * connect is needed; {@code newPlainText} only allocates the client. + */ + @Test + public void testClientGetterReturnsConstructorInstance() { + try (QwpQueryClient client = QwpQueryClient.newPlainText("localhost", 9000)) { + QueryWorker worker = new QueryWorker(client, null, 0); + Assert.assertSame("client() must return the instance passed to the constructor", + client, worker.client()); + // Idempotent across calls -- the field is final. + Assert.assertSame(worker.client(), worker.client()); + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java new file mode 100644 index 00000000..d6f33a2b --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java @@ -0,0 +1,258 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.impl; + +import io.questdb.client.Sender; +import io.questdb.client.cutlass.line.LineSenderException; +import io.questdb.client.impl.SenderPool; +import org.junit.Assert; +import org.junit.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Unit tests for the {@link SenderPool} borrow/return semantics. Uses the + * {@code http} schema with a dead address: HTTP Senders connect lazily on + * first request, so the pool builds without I/O and the tests can exercise + * borrow/return without needing a real server. + *

+ * Tests never call methods that would attempt a network round-trip + * (no {@code flush}, no row builders that auto-flush). Pooled + * {@link io.questdb.client.impl.PooledSender#close()} does call + * {@code delegate.flush()}, but on an empty buffer that path is a no-op for + * HTTP transport. + */ +public class SenderPoolTest { + + private static final String DEAD_HTTP_CONFIG = + "http::addr=127.0.0.1:1;protocol_version=2;auto_flush=off;"; + + @Test + public void testBorrowReturnRecyclesSameDecorator() { + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + Sender first = pool.borrow(); + first.close(); + Sender second = pool.borrow(); + Assert.assertSame("returned decorator should be reused after close()", first, second); + second.close(); + } + } + + @Test + public void testCloseIdempotent() { + SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 2, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + pool.close(); + pool.close(); + } + + @Test + public void testCloseRejectsSubsequentBorrow() { + SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + pool.close(); + try { + pool.borrow(); + Assert.fail("borrow after close must throw"); + } catch (LineSenderException ignored) { + } + } + + @Test + public void testExhaustionTimeoutThrows() { + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 100, Long.MAX_VALUE, Long.MAX_VALUE)) { + long start = System.nanoTime(); + try (Sender ignored = pool.borrow()) { + pool.borrow(); + Assert.fail("expected timeout"); + } catch (LineSenderException e) { + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + Assert.assertTrue("error should mention timeout, was: " + e.getMessage(), + e.getMessage().contains("timed out")); + Assert.assertTrue("should have waited close to the timeout, elapsed=" + elapsedMs, + elapsedMs >= 90); + } + } + } + + @Test + public void testPoolBuildsRequestedNumberOfSenders() { + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 3, 3, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + Sender a = pool.borrow(); + Sender b = pool.borrow(); + Sender c = pool.borrow(); + Assert.assertNotSame(a, b); + Assert.assertNotSame(b, c); + Assert.assertNotSame(a, c); + a.close(); + b.close(); + c.close(); + } + } + + @Test + public void testElasticGrowsUpToMax() { + // min=1, max=3 -- starts at 1, grows on demand to 3. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 3, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + Assert.assertEquals("pre-warm to min", 1, pool.totalSize()); + Sender a = pool.borrow(); + Assert.assertEquals(1, pool.totalSize()); + Sender b = pool.borrow(); + Assert.assertEquals("borrowing past min grows", 2, pool.totalSize()); + Sender c = pool.borrow(); + Assert.assertEquals(3, pool.totalSize()); + // At max: next borrow times out. + try { + pool.borrow(); + Assert.fail("4th borrow must time out at max=3"); + } catch (LineSenderException ignored) { + } + a.close(); + b.close(); + c.close(); + Assert.assertEquals("size unchanged on return", 3, pool.totalSize()); + } + } + + @Test + public void testAvailableSizeTracksBorrowAndReturn() throws InterruptedException { + // min=2, max=4. Walk the full lifecycle and assert availableSize() and + // totalSize() stay in sync at every step: pre-warm, borrow shrinks + // available, growth doesn't change available (the new slot goes + // straight to the caller), return restores availability, reap shrinks + // total back toward min but never below. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 4, 1_000, 100, Long.MAX_VALUE)) { + // Pre-warmed to min=2; everything is idle. + Assert.assertEquals(2, pool.totalSize()); + Assert.assertEquals(2, pool.availableSize()); + + // Borrowing from the warm slots leaves total unchanged but consumes one available. + Sender a = pool.borrow(); + Assert.assertEquals(2, pool.totalSize()); + Assert.assertEquals(1, pool.availableSize()); + + Sender b = pool.borrow(); + Assert.assertEquals(2, pool.totalSize()); + Assert.assertEquals(0, pool.availableSize()); + + // Borrowing past min grows the pool. The new slot goes straight to + // the caller, so availableSize stays at 0. + Sender c = pool.borrow(); + Assert.assertEquals(3, pool.totalSize()); + Assert.assertEquals(0, pool.availableSize()); + + // Returning two restores availability without touching total. + a.close(); + b.close(); + Assert.assertEquals(3, pool.totalSize()); + Assert.assertEquals(2, pool.availableSize()); + + // Reaping idle slots over min closes them; available counts the + // remaining idle ones. Total shrinks; min=2 is respected so we end + // up with min=2 total and (min - in-use)=1 available. + Thread.sleep(150); + pool.reapIdle(); + Assert.assertEquals(2, pool.totalSize()); + Assert.assertEquals(1, pool.availableSize()); + + c.close(); + Assert.assertEquals(2, pool.totalSize()); + Assert.assertEquals(2, pool.availableSize()); + } + } + + @Test + public void testAvailableSizeZeroAfterClose() { + SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 2, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + Assert.assertEquals(2, pool.availableSize()); + pool.close(); + // close() destroys every underlying Sender; the available queue is no + // longer being added to, but the snapshot read is still safe. The + // exact value (0 or stale) is less important than the call not + // throwing on a closed pool. + int snapshot = pool.availableSize(); + Assert.assertTrue("availableSize on closed pool must be a non-negative snapshot, got " + snapshot, + snapshot >= 0); + } + + @Test + public void testReapIdleShrinksToMin() throws InterruptedException { + // Short idle timeout; reapIdle() drives the sweep deterministically. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 3, 1_000, 100, Long.MAX_VALUE)) { + Sender a = pool.borrow(); + Sender b = pool.borrow(); + Sender c = pool.borrow(); + Assert.assertEquals(3, pool.totalSize()); + a.close(); + b.close(); + c.close(); + // All idle; wait until idle threshold passes, then sweep. + Thread.sleep(150); + pool.reapIdle(); + Assert.assertEquals("reap must shrink to min", 1, pool.totalSize()); + } + } + + @Test + public void testReapIdleRespectsMinSize() throws InterruptedException { + // min=2: two slots must stay even after long idle. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 4, 1_000, 50, Long.MAX_VALUE)) { + Sender a = pool.borrow(); + Sender b = pool.borrow(); + Sender c = pool.borrow(); + a.close(); + b.close(); + c.close(); + Thread.sleep(100); + pool.reapIdle(); + Assert.assertEquals("min=2 must be preserved", 2, pool.totalSize()); + } + } + + @Test + public void testThreadAffinityIsPerThread() throws InterruptedException { + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 2, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + Sender mainPinned = pool.pinToCurrentThread(); + Assert.assertSame("re-pin on same thread returns same instance", + mainPinned, pool.pinToCurrentThread()); + + AtomicReference otherPinned = new AtomicReference<>(); + CountDownLatch done = new CountDownLatch(1); + Thread t = new Thread(() -> { + try { + otherPinned.set(pool.pinToCurrentThread()); + } finally { + done.countDown(); + } + }); + t.start(); + Assert.assertTrue(done.await(2, TimeUnit.SECONDS)); + Assert.assertNotSame("different threads must get different pinned Senders", + mainPinned, otherPinned.get()); + + pool.releaseCurrentThread(); + } + } +} From 6dda90a29bb5f0035471ef90173e82aa71a777ec Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 04:42:30 +0100 Subject: [PATCH 02/12] Replace reflection in QwpWebSocketSenderTest with @TestOnly seams QwpWebSocketSenderTest reached into QwpWebSocketSender via getDeclaredField/getDeclaredMethod for five members. Restoring totalBufferedBytes after an accidental delete made the reflection visible -- removing it altogether is the better fix. QwpWebSocketSender changes: - applyServerBatchSizeLimit promoted to @TestOnly public (production callers still use it; the annotation flags it as a test seam too). - getPendingBytes promoted to @TestOnly public, deduped against the former private getter (the one internal caller at the auto-flush guard now binds to the public version). - New @TestOnly public getters: getEffectiveAutoFlushBytes, getServerMaxBatchSize. - New @TestOnly public setConnectedForTest(boolean) so tests can fake the post-handshake state without reflection. - totalBufferedBytes restored as @TestOnly public (kept by the test and QwpTotalBufferedBytesBenchmark; otherwise dead). QwpWebSocketSenderTest: - Five reflection-based static helpers (~50 LOC) deleted. - Three java.lang.reflect imports removed. - All call sites now go through direct method invocation. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../qwp/client/QwpWebSocketSender.java | 70 ++++++++++++- .../qwp/client/QwpWebSocketSenderTest.java | 99 ++++--------------- 2 files changed, 84 insertions(+), 85 deletions(-) diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java index f59dcb4b..8f01c6e2 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java @@ -1520,6 +1520,12 @@ public long getDroppedErrorNotifications() { return d == null ? 0L : d.getDroppedNotifications(); } + /** Returns the live byte budget the auto-flush path actually enforces. */ + @TestOnly + public int getEffectiveAutoFlushBytes() { + return effectiveAutoFlushBytes; + } + /** * Snapshot of the typed payload for the latched terminal server-rejection error, * or {@code null} if the I/O loop has not latched a server-rejection terminal @@ -1546,6 +1552,26 @@ public int getOrAddGlobalSymbol(CharSequence symbol) { return globalId; } + /** + * Running tally the row builder maintains so auto-flush thresholds can be + * evaluated without re-walking every table per row. Exposed for tests + * that compare this incremental counter against a ground-truth walk. + */ + @TestOnly + public long getPendingBytes() { + return pendingBytes; + } + + /** + * Server-advertised cap on the per-batch raw byte size. Zero before the + * first connect; updated by every successful reconnect via + * {@link #applyServerBatchSizeLimit(int)}. + */ + @TestOnly + public int getServerMaxBatchSize() { + return serverMaxBatchSize; + } + @TestOnly public QwpTableBuffer getTableBuffer(String tableName) { QwpTableBuffer buffer = tableBuffers.get(tableName); @@ -1928,6 +1954,17 @@ public void reset() { * publishing or reconnect. See {@link SenderConnectionListener} for the * full delivery contract. */ + /** + * Forces the {@code connected} flag without going through the real + * connect handshake. Lets unit tests exercise post-connect code paths + * (auto-flush bookkeeping, batch-size guards, ack tracking) on a + * sender that never opened a socket. Never call from production code. + */ + @TestOnly + public void setConnectedForTest(boolean connected) { + this.connected = connected; + } + public void setConnectionListener(SenderConnectionListener listener) { SenderConnectionListener effective = listener != null ? listener : DefaultSenderConnectionListener.INSTANCE; this.connectionListener = effective; @@ -2277,7 +2314,8 @@ private static List singleEndpoint(String host, int port) { * preserved even when the server advertises a cap, so applications that * opted out keep the contract they asked for. */ - private void applyServerBatchSizeLimit(int advertisedMaxBatchSize) { + @TestOnly + public void applyServerBatchSizeLimit(int advertisedMaxBatchSize) { serverMaxBatchSize = advertisedMaxBatchSize; if (autoFlushBytes <= 0) { // User opted out of byte-based auto-flush; respect that even when @@ -2952,10 +2990,6 @@ private void flushPendingRows() { firstPendingRowTimeNanos = 0; } - private long getPendingBytes() { - return pendingBytes; - } - private void resetSchemaStateForNewConnection() { maxSentSchemaId = -1; nextSchemaId = 0; @@ -3133,6 +3167,32 @@ private long toMicros(long value, ChronoUnit unit) { } } + /** + * Total buffered bytes across every per-table column buffer. Sums the + * tableBuffers map with the same null-tolerant walk the old sendRow path + * used. Currently dead in production code (the sendRow accounting was + * inlined for tighter bookkeeping) but kept so + * {@code QwpWebSocketSenderTest} can exercise the walk shape directly + * and {@code QwpTotalBufferedBytesBenchmark} can quote it as the + * baseline it benchmarks. Removing it would break both. + */ + @TestOnly + public long totalBufferedBytes() { + long total = 0; + ObjList keys = tableBuffers.keys(); + for (int i = 0, n = keys.size(); i < n; i++) { + CharSequence tableName = keys.getQuick(i); + if (tableName == null) { + continue; + } + QwpTableBuffer tb = tableBuffers.get(tableName); + if (tb != null) { + total += tb.getBufferedBytes(); + } + } + return total; + } + private void validateTableName(CharSequence name) { if (name == null || !TableUtils.isValidTableName(name, MAX_TABLE_NAME_LENGTH)) { if (name == null || name.length() == 0) { diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderTest.java index e26d30f4..ecc8eed5 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderTest.java @@ -36,9 +36,6 @@ import org.junit.Assert; import org.junit.Test; -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -60,10 +57,10 @@ public void testApplyServerBatchSizeLimit_advertisedClampsLargerConfigured() thr /*autoFlushIntervalNanos*/ 0L)) { // Server advertises 16 MB. Configured 32 MB is over the cap, // so the effective budget should drop to 90% of 16 MB. - invokeApplyServerBatchSizeLimit(sender, 16 * 1024 * 1024); - int effective = getEffectiveAutoFlushBytes(sender); + sender.applyServerBatchSizeLimit(16 * 1024 * 1024); + int effective = sender.getEffectiveAutoFlushBytes(); Assert.assertEquals((long) (16 * 1024 * 1024) * 9 / 10, effective); - Assert.assertEquals(16 * 1024 * 1024, getServerMaxBatchSize(sender)); + Assert.assertEquals(16 * 1024 * 1024, sender.getServerMaxBatchSize()); } }); } @@ -78,9 +75,9 @@ public void testApplyServerBatchSizeLimit_advertisedZeroKeepsConfigured() throws /*autoFlushIntervalNanos*/ 0L)) { // 0 advertisement = older server. Effective budget must equal // the configured value verbatim so the sender keeps working. - invokeApplyServerBatchSizeLimit(sender, 0); - Assert.assertEquals(2 * 1024 * 1024, getEffectiveAutoFlushBytes(sender)); - Assert.assertEquals(0, getServerMaxBatchSize(sender)); + sender.applyServerBatchSizeLimit(0); + Assert.assertEquals(2 * 1024 * 1024, sender.getEffectiveAutoFlushBytes()); + Assert.assertEquals(0, sender.getServerMaxBatchSize()); } }); } @@ -95,8 +92,8 @@ public void testApplyServerBatchSizeLimit_configuredSmallerThanAdvertisedWins() /*autoFlushIntervalNanos*/ 0L)) { // Server advertises 16 MB; configured 2 MB is well below. // Keep the user's tighter budget rather than overriding it. - invokeApplyServerBatchSizeLimit(sender, 16 * 1024 * 1024); - Assert.assertEquals(2 * 1024 * 1024, getEffectiveAutoFlushBytes(sender)); + sender.applyServerBatchSizeLimit(16 * 1024 * 1024); + Assert.assertEquals(2 * 1024 * 1024, sender.getEffectiveAutoFlushBytes()); } }); } @@ -112,9 +109,9 @@ public void testApplyServerBatchSizeLimit_optOutPreservedDespiteAdvertisement() // User explicitly disabled the byte trigger. The server's // advertised cap must update serverMaxBatchSize (for the // single-row guard) but must not re-enable byte flushing. - invokeApplyServerBatchSizeLimit(sender, 16 * 1024 * 1024); - Assert.assertEquals(0, getEffectiveAutoFlushBytes(sender)); - Assert.assertEquals(16 * 1024 * 1024, getServerMaxBatchSize(sender)); + sender.applyServerBatchSizeLimit(16 * 1024 * 1024); + Assert.assertEquals(0, sender.getEffectiveAutoFlushBytes()); + Assert.assertEquals(16 * 1024 * 1024, sender.getServerMaxBatchSize()); } }); } @@ -529,7 +526,7 @@ public void testPendingBytesMatchesGroundTruthAcrossTableSwitches() throws Excep // Bypass ensureConnected: sendRow only short-circuits on // connected==true and we don't drive any path that touches // the cursor engine in this test. - setConnected(sender); + sender.setConnectedForTest(true); // Round-robin three tables to exercise the per-row delta // logic across switches. The running pendingBytes must @@ -541,8 +538,8 @@ public void testPendingBytesMatchesGroundTruthAcrossTableSwitches() throws Excep sender.atNow(); Assert.assertEquals( "row " + i + ": pendingBytes diverged from ground truth", - invokeTotalBufferedBytes(sender), - getPendingBytes(sender)); + sender.totalBufferedBytes(), + sender.getPendingBytes()); } } }); @@ -556,19 +553,19 @@ public void testPendingBytesUnchangedByCancelRow() throws Exception { /*autoFlushRows*/ Integer.MAX_VALUE, /*autoFlushBytes*/ 0, /*autoFlushIntervalNanos*/ 0L)) { - setConnected(sender); + sender.setConnectedForTest(true); sender.table("t0").longColumn("a", 1).longColumn("b", 2).atNow(); - long committed = getPendingBytes(sender); - Assert.assertEquals(invokeTotalBufferedBytes(sender), committed); + long committed = sender.getPendingBytes(); + Assert.assertEquals(sender.totalBufferedBytes(), committed); // Begin a row, then cancel. The committed bytes must not // change and pendingBytes must still match ground truth. sender.table("t0").longColumn("a", 99); sender.cancelRow(); - Assert.assertEquals(committed, getPendingBytes(sender)); - Assert.assertEquals(invokeTotalBufferedBytes(sender), getPendingBytes(sender)); + Assert.assertEquals(committed, sender.getPendingBytes()); + Assert.assertEquals(sender.totalBufferedBytes(), sender.getPendingBytes()); } }); } @@ -708,64 +705,6 @@ private static void assertClosed(Runnable r) { } } - private static int getEffectiveAutoFlushBytes(QwpWebSocketSender sender) throws Exception { - Field field = QwpWebSocketSender.class.getDeclaredField("effectiveAutoFlushBytes"); - field.setAccessible(true); - return field.getInt(sender); - } - - private static long getPendingBytes(QwpWebSocketSender sender) throws Exception { - Field field = QwpWebSocketSender.class.getDeclaredField("pendingBytes"); - field.setAccessible(true); - return field.getLong(sender); - } - - private static int getServerMaxBatchSize(QwpWebSocketSender sender) throws Exception { - Field field = QwpWebSocketSender.class.getDeclaredField("serverMaxBatchSize"); - field.setAccessible(true); - return field.getInt(sender); - } - - private static void invokeApplyServerBatchSizeLimit(QwpWebSocketSender sender, int advertised) throws Exception { - Method m = QwpWebSocketSender.class.getDeclaredMethod("applyServerBatchSizeLimit", int.class); - m.setAccessible(true); - try { - m.invoke(sender, advertised); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - if (cause instanceof Exception) { - throw (Exception) cause; - } - if (cause instanceof Error) { - throw (Error) cause; - } - throw new RuntimeException(cause); - } - } - - private static long invokeTotalBufferedBytes(QwpWebSocketSender sender) throws Exception { - Method m = QwpWebSocketSender.class.getDeclaredMethod("totalBufferedBytes"); - m.setAccessible(true); - try { - return (long) m.invoke(sender); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - if (cause instanceof Exception) { - throw (Exception) cause; - } - if (cause instanceof Error) { - throw (Error) cause; - } - throw new RuntimeException(cause); - } - } - - private static void setConnected(QwpWebSocketSender sender) throws Exception { - Field field = QwpWebSocketSender.class.getDeclaredField("connected"); - field.setAccessible(true); - field.setBoolean(sender, true); - } - /** * Creates a sender without connecting. * For unit tests that don't need actual connectivity. From 911a056c0b0c88c7281a34f4a7381944b2ed1996 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 06:14:18 +0100 Subject: [PATCH 03/12] Make SegmentRing sort regression test deterministic testLargeSegmentCountReopensInOrder used to assert wall-clock elapsed < 5s as a proxy for "sortByBaseSeq stayed O(N log N)". The bound was generous for any modern machine but tight enough to flake on loaded Windows CI runners doing 2048 mmap-creates + 2048 mmap-opens. Replace the timing assertion with a comparison-count assertion: - SegmentRing tracks the total baseSeq comparisons performed by sortByBaseSeq in a static counter (one += per partition pass, bumping by 3 + (hi - lo - 1) to cover median-of-three plus the partition loop). - @TestOnly public accessors getSortComparisons() / resetSortComparisons() let the test reset before the run and read after. - testLargeSegmentCountReopensInOrder now asserts comparisons < 5 * N * log2(N), which sits about 30x below the O(N^2) regression value and 1.5x above the expected O(N log N) count. Detects a true regression on first commit, deterministic across hardware. Production cost is one volatile-free += per partition pass, dwarfed by the mmap I/O the same path does on every segment. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../qwp/client/sf/cursor/SegmentRing.java | 35 ++++++++++++++++++- .../qwp/client/sf/cursor/SegmentRingTest.java | 28 +++++++++------ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java index e4eafb47..09d91888 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java @@ -27,6 +27,7 @@ import io.questdb.client.std.Files; import io.questdb.client.std.ObjList; import io.questdb.client.std.QuietCloseable; +import org.jetbrains.annotations.TestOnly; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,6 +63,13 @@ public final class SegmentRing implements QuietCloseable { /** Sentinel: append failed because the payload doesn't fit in a fresh segment. */ public static final long PAYLOAD_TOO_LARGE = -2L; private static final Logger LOG = LoggerFactory.getLogger(SegmentRing.class); + // Tally of baseSeq comparisons performed by sortByBaseSeq across every + // openExisting() recovery on this JVM. Used by SegmentRingTest to + // assert the sort stays O(N log N) without relying on wall-clock time + // (CI runner variance makes elapsed-millisecond bounds flaky). Cheap + // in production: one volatile-free add per partition pass, dwarfed by + // the mmap I/O the recovery does on every segment. + private static long sortComparisons; private final long maxBytesPerSegment; // Sealed segments in baseSeq order, oldest first. Active is held separately. // Single-writer (producer thread, on rotation); single-reader at trim time @@ -652,6 +660,27 @@ public synchronized long totalSegmentBytes() { return total; } + /** + * Returns the cumulative count of baseSeq comparisons performed by + * {@link #sortByBaseSeq} since the last {@link #resetSortComparisons()} + * (or process start). The count is incremented once per partition pass + * for the median-of-three pivot pick plus once per element compared + * against the pivot, so a clean run on N segments adds roughly + * {@code 3 + (hi - lo - 1)} per recursive frame, summing to O(N log N). + * Exposed for {@code SegmentRingTest} to detect O(N²) regressions + * deterministically. + */ + @TestOnly + public static long getSortComparisons() { + return sortComparisons; + } + + /** Zeroes the counter exposed via {@link #getSortComparisons()}. */ + @TestOnly + public static void resetSortComparisons() { + sortComparisons = 0; + } + /** * In-place quicksort over {@code list[lo, hi)} keyed by ascending * {@code baseSeq}. Median-of-three pivot avoids the pathological O(N²) @@ -666,7 +695,11 @@ private static void sortByBaseSeq(ObjList list, int lo, int hi) { long a = list.get(lo).baseSeq(); long b = list.get(mid).baseSeq(); long c = list.get(hi - 1).baseSeq(); - // Median of {a, b, c} → pivot index. + // Median of {a, b, c} → pivot index. Three compareUnsigned calls + // worst case; bumping by a constant 3 keeps the counter cheap and + // still strictly upper-bounds the true work (some short-circuit + // out after 1-2 compares). + sortComparisons += 3L + (hi - lo - 1); int pivotIdx; if (Long.compareUnsigned(a, b) < 0) { if (Long.compareUnsigned(b, c) < 0) pivotIdx = mid; diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentRingTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentRingTest.java index 0043e521..f1a0fcde 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentRingTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentRingTest.java @@ -565,10 +565,10 @@ public void testLargeSegmentCountReopensInOrder() throws Exception { } } - long startMs = System.currentTimeMillis(); + SegmentRing.resetSortComparisons(); try (SegmentRing ring = SegmentRing.openExisting(tmpDir, MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 16)) { - long elapsed = System.currentTimeMillis() - startMs; + long comparisons = SegmentRing.getSortComparisons(); assertNotNull("recovery must produce a ring", ring); // After recovery, the ring's nextSeqHint is one past the // last frame on disk. With one frame per segment numbered @@ -577,14 +577,22 @@ public void testLargeSegmentCountReopensInOrder() throws Exception { n, ring.nextSeqHint()); // publishedFsn = n - 1 (last frame visible). assertEquals(n - 1, ring.publishedFsn()); - // 5s is comfortably above the quicksort path (sub-second on - // any modern machine) and well below the seconds-of-CPU the - // production-ceiling O(N²) regression would produce. Tight - // enough to fire if the algorithm regresses, loose enough - // to survive a slow CI runner. - assertTrue("recovery took " + elapsed + " ms (expected < 5000); " - + "regression suggests the segment sort is back to O(N²)", - elapsed < 5_000); + // O(N log N) quicksort with good pivots does ~2-3 * N * log2(N) + // comparisons; the partition-pass + median-of-three counter we + // increment per recursive frame upper-bounds this at roughly + // 3 N log2(N). The naive O(N²) regression on already-sorted + // input would do ~N(N-1)/2 -- 2.1M at N=2048 vs the ~67k a + // healthy sort produces. The 5x N log2(N) bound below sits + // about 30x below the O(N²) value and 1.5x above the + // expected count, so it fires on a real regression without + // flapping on harmless implementation drift. Comparison + // counts are deterministic across CI hardware, unlike the + // wall-clock bound this assertion used to carry. + long bound = 5L * n * (long) (Math.log(n) / Math.log(2)); + assertTrue("sort took " + comparisons + " comparisons (expected < " + bound + + " = 5 * N * log2(N) for N=" + n + "); " + + "regression suggests the segment sort is back to O(N^2)", + comparisons < bound); } } finally { Unsafe.free(buf, 16, MemoryTag.NATIVE_DEFAULT); From 3d8182d68a5124f591b42ce8659ffcf6f28a1269 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 14:26:41 +0100 Subject: [PATCH 04/12] Fix three SenderPool / QwpQueryClient lifecycle bugs Three independent fixes to the new QWP pooling layer surfaced by code review against the design/egress-api-ergonomics-review notes. SenderPool.close() left ThreadLocal references on every thread that had previously called pinToCurrentThread(). ThreadLocal#remove only clears the calling thread's slot, so a subsequent db.sender() on a different thread took the cached-pin early-return and handed back a wrapper whose delegate had just been closed by close(). The next operation on it dereferenced a closed native handle. Add an invalidated flag on PooledSender that close() flips on every wrapper under the lock; pinToCurrentThread() and releaseCurrentThread() check the flag and drop the stale ThreadLocal slot so borrow() reports "QuestDB handle is closed" instead. PooledSender.close() returned broken wrappers to the pool when delegate.flush() threw. The Sender contract intentionally does NOT clear its buffer on flush failure (to permit retry), so the next borrower inherited the failed rows; on WebSocket transport the delegate itself was terminally broken. Route the failure path through a new SenderPool.discardBroken() that marks the wrapper invalidated, removes it from the pool under the lock, signals one waiter, and closes the delegate outside the lock. The invalidated flag does double duty: a broken thread-pinned slot is also rejected by pinToCurrentThread() so the next db.sender() re-borrows a fresh slot. QwpQueryClient.cancel() between submit() returning and executeOnce() writing currentRequestId was a silent no-op: cancel() read -1 and skipped the wire-send. The window covers worker-wakeup latency, bind encoding (user code), and every failover retry's backoff. Latch a pendingCancel flag in cancel() before reading currentRequestId; execute() clears it once at the outermost entry; executeOnce() reads it immediately after the currentRequestId assignment and re-issues io.requestCancel() if set. The latch is intentionally not cleared inside executeOnce(), so failover retries also honor the cancel. Tests: - SenderPoolTest: testPinAfterCloseRejectsStaleEntry, testReleaseAfterCloseIsSafe, testBrokenSenderIsNotReturnedToPool. - QwpQueryClientUnitTest: testCancelDuringDispatchWindowLatchesPendingIntent, testExecuteEntryClearsStalePendingCancel, backed by a new @TestOnly isPendingCancelForTest seam. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cutlass/qwp/client/QwpQueryClient.java | 40 ++++++++ .../io/questdb/client/impl/PooledSender.java | 24 ++++- .../io/questdb/client/impl/SenderPool.java | 43 ++++++++- .../qwp/client/QwpQueryClientUnitTest.java | 40 ++++++++ .../client/test/impl/SenderPoolTest.java | 93 +++++++++++++++++++ 5 files changed, 238 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java index 64f0dfd5..5c5227b1 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java @@ -35,6 +35,7 @@ import io.questdb.client.std.QuietCloseable; import io.questdb.client.std.Zstd; import io.questdb.client.std.str.StringSink; +import org.jetbrains.annotations.TestOnly; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -252,6 +253,14 @@ public class QwpQueryClient implements QuietCloseable { // re-clamp) so a misconfigured server is observable from user code. private int negotiatedZstdLevel; private long nextRequestId = 1; + // Cancel intent latched between {@link #cancel} and the point where + // {@link #executeOnce} assigns {@link #currentRequestId}. Without this + // latch, a cancel arriving in the dispatch window (after the user thread's + // submit() returned but before the worker reached the requestId write) is + // dropped silently because cancel()'s wire-send is gated on a non-negative + // currentRequestId. Volatile so a cancel from any thread is visible to the + // worker thread's post-requestId read. + private volatile boolean pendingCancel; // Decoded SERVER_INFO from the current connection's handshake. Null before // connect() has succeeded, and on connections that negotiated v1 (which // doesn't emit the frame). Volatile so the I/O thread's read on the @@ -713,6 +722,13 @@ public static QwpQueryClient newPlainText(CharSequence host, int port) { * handler's {@code onError} (on the execute-ing thread) will see it. */ public void cancel() { + // Latch FIRST so a cancel arriving in the dispatch window (after + // execute() cleared the latch but before executeOnce() wrote + // currentRequestId) is observed on the worker thread's + // post-requestId read. Without this, cancel() reads + // currentRequestId == -1, the wire-send is skipped, and the user's + // intent is silently dropped. + pendingCancel = true; QwpEgressIoThread io = ioThread; long id = currentRequestId; if (io != null && id >= 0L) { @@ -967,6 +983,12 @@ public void execute(String sql, QwpBindSetter binds, QwpColumnBatchHandler handl throw new IllegalStateException( "QwpQueryClient.execute called while another execute is in flight; one query at a time per client"); } + // Drop any cancel latched between calls (e.g., a watchdog that fired + // while no request was in flight, or a previous pooled user that + // released the client without ever calling execute). Failover retries + // inside this execute() must still honor a fresh cancel, so the latch + // is intentionally NOT cleared inside executeOnce(). + pendingCancel = false; try { executeImpl(sql, binds, handler); } finally { @@ -1051,6 +1073,17 @@ public boolean isConnected() { return connected; } + /** + * Test-only view of the dispatch-window cancel latch. Returns {@code true} + * when {@link #cancel} was invoked after the outermost {@link #execute} + * cleared the latch and before {@link #execute} has consumed it on the + * {@code currentRequestId = requestId} write. + */ + @TestOnly + public boolean isPendingCancelForTest() { + return pendingCancel; + } + /** * Test-only / diagnostics hook: injects a synthetic terminal failure * through the current generation's listener. Production code does not @@ -1753,6 +1786,13 @@ private void executeOnce(String sql, QwpBindSetter binds, FailoverProbeHandler p } long requestId = nextRequestId++; currentRequestId = requestId; + // Honor a cancel that arrived during the dispatch window. The latch + // is intentionally not cleared here: if this attempt fails over, + // the retry must also be cancelled. The latch is cleared once at + // the outermost execute() entry. + if (pendingCancel) { + io.requestCancel(requestId); + } try { io.submitQuery(sql, requestId, initialCreditBytes, bindValues.count(), bindValues.bufferPtr(), bindValues.bufferLen()); while (true) { diff --git a/core/src/main/java/io/questdb/client/impl/PooledSender.java b/core/src/main/java/io/questdb/client/impl/PooledSender.java index 9346967c..3cf2ec98 100644 --- a/core/src/main/java/io/questdb/client/impl/PooledSender.java +++ b/core/src/main/java/io/questdb/client/impl/PooledSender.java @@ -52,6 +52,7 @@ public final class PooledSender implements Sender { private final SenderPool pool; private volatile long idleSinceMillis; private volatile boolean inUse; + private volatile boolean invalidated; PooledSender(Sender delegate, SenderPool pool) { this.delegate = delegate; @@ -138,11 +139,24 @@ public void close() { if (!inUse) { return; } + boolean broken = false; try { delegate.flush(); + } catch (RuntimeException e) { + // Sender does not clear its buffer on flush failure (see + // Sender Javadoc), and WebSocket transport latches the failure + // for good. Either way, the wrapper is unsafe to recycle: the + // next borrower would inherit the failed rows or a dead + // connection. + broken = true; + throw e; } finally { inUse = false; - pool.giveBack(this); + if (broken) { + pool.discardBroken(this); + } else { + pool.giveBack(this); + } } } @@ -350,6 +364,10 @@ boolean isInUse() { return inUse; } + boolean isInvalidated() { + return invalidated; + } + void markIdleAt(long nowMillis) { idleSinceMillis = nowMillis; } @@ -357,4 +375,8 @@ void markIdleAt(long nowMillis) { void markInUse() { inUse = true; } + + void markInvalidated() { + invalidated = true; + } } diff --git a/core/src/main/java/io/questdb/client/impl/SenderPool.java b/core/src/main/java/io/questdb/client/impl/SenderPool.java index 8eaae97a..c463f948 100644 --- a/core/src/main/java/io/questdb/client/impl/SenderPool.java +++ b/core/src/main/java/io/questdb/client/impl/SenderPool.java @@ -179,6 +179,14 @@ public void close() { return; } closed = true; + // Mark every pooled wrapper invalidated so pinToCurrentThread() + // on other threads -- which never takes this lock -- can detect + // that its cached entry no longer wraps a live delegate. Removing + // the calling thread's ThreadLocal only clears one slot; other + // threads' slots survive until they read the flag. + for (int i = 0; i < all.size(); i++) { + all.get(i).markInvalidated(); + } threadAffine.remove(); slotReleased.signalAll(); } finally { @@ -194,6 +202,32 @@ public void close() { } } + /** + * Evicts a slot whose delegate has failed (typically a {@code flush()} + * failure observed in {@link PooledSender#close()}). The wrapper is + * marked invalidated so any thread-pinned reference gets rejected on the + * next {@link #pinToCurrentThread()} call; the slot is removed from + * {@code all} so the pool can grow back into a fresh slot on demand. The + * underlying delegate is closed outside the lock so a slow real-close + * does not stall other borrowers. + */ + void discardBroken(PooledSender s) { + s.markInvalidated(); + lock.lock(); + try { + all.remove(s); + // Wake one waiter -- the cap check in borrow() uses all.size(), + // so a freed slot may now allow a creation attempt. + slotReleased.signal(); + } finally { + lock.unlock(); + } + try { + s.delegate().close(); + } catch (RuntimeException ignored) { + } + } + public void giveBack(PooledSender s) { long now = System.currentTimeMillis(); s.markIdleAt(now); @@ -212,9 +246,12 @@ public void giveBack(PooledSender s) { public PooledSender pinToCurrentThread() { PooledSender pinned = threadAffine.get(); - if (pinned != null) { + if (pinned != null && !pinned.isInvalidated()) { return pinned; } + if (pinned != null) { + threadAffine.remove(); + } PooledSender s = borrow(); threadAffine.set(s); return s; @@ -291,6 +328,10 @@ public void releaseCurrentThread() { return; } threadAffine.remove(); + if (pinned.isInvalidated()) { + // Pool was closed: delegate is already closed, skip flush/giveBack. + return; + } pinned.close(); } diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpQueryClientUnitTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpQueryClientUnitTest.java index f5ee7bdb..4456949a 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpQueryClientUnitTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpQueryClientUnitTest.java @@ -124,6 +124,46 @@ public void testExecuteWithBindsBeforeConnectThrowsIllegalState() { } } + @Test + public void testCancelDuringDispatchWindowLatchesPendingIntent() { + // The dispatch window is the gap between the user thread's submit() + // returning and the worker thread reaching the + // `currentRequestId = requestId` write inside executeOnce(). A cancel() + // during that window currently sees currentRequestId == -1 and returns + // silently; the user's intent is lost. With the latch in place, + // cancel() must record the pending intent so executeOnce() honors it + // as soon as the request id is assigned. + try (QwpQueryClient c = QwpQueryClient.newPlainText("localhost", 9000)) { + Assert.assertFalse("a fresh client must not start with a pending cancel", + c.isPendingCancelForTest()); + c.cancel(); + Assert.assertTrue("cancel() before currentRequestId is assigned must latch the intent", + c.isPendingCancelForTest()); + } + } + + @Test + public void testExecuteEntryClearsStalePendingCancel() { + // A cancel latched on this client (e.g., by a previous user that + // returned the client to a pool without ever submitting) must not + // bleed into the next execute() call. execute() must clear the latch + // at entry, even when the call ultimately fails on the + // "not connected" guard inside executeImpl(). + try (QwpQueryClient c = QwpQueryClient.newPlainText("localhost", 9000)) { + c.cancel(); + Assert.assertTrue("precondition: latch is set", + c.isPendingCancelForTest()); + try { + c.execute("SELECT 1", NOOP_HANDLER); + Assert.fail("execute() before connect() must throw"); + } catch (IllegalStateException expected) { + // expected: client never connected + } + Assert.assertFalse("execute() entry must clear stale pending cancel", + c.isPendingCancelForTest()); + } + } + @Test public void testCancelOnFreshClientIsNoOp() { // cancel() must be safe to call before connect (e.g., a watchdog timer diff --git a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java index d6f33a2b..a5b043a3 100644 --- a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java +++ b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java @@ -62,6 +62,35 @@ public void testBorrowReturnRecyclesSameDecorator() { } } + @Test + public void testBrokenSenderIsNotReturnedToPool() { + // Borrowing, buffering a row, and then closing forces flush() against + // the unreachable address, which throws. The broken wrapper must not + // be returned to the pool: its delegate's buffer still holds the + // failed row, and on transports with terminal-failure semantics the + // delegate is also unusable. Either way, the next borrower must get + // a fresh wrapper. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + Sender first = pool.borrow(); + first.table("t").longColumn("v", 1).atNow(); + try { + first.close(); + Assert.fail("close() with buffered rows against an unreachable host must throw"); + } catch (LineSenderException ignored) { + // expected + } + Sender second = pool.borrow(); + try { + Assert.assertNotSame("broken sender must not be handed back to next borrower", + first, second); + } finally { + if (second != first) { + second.close(); + } + } + } + } + @Test public void testCloseIdempotent() { SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 2, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); @@ -231,6 +260,70 @@ public void testReapIdleRespectsMinSize() throws InterruptedException { } } + @Test + public void testPinAfterCloseRejectsStaleEntry() throws Exception { + // Pin from a worker thread, close the pool from main. The worker's + // ThreadLocal still references its PooledSender, but the underlying + // delegate has been closed. The next pinToCurrentThread() on the + // worker must reject the stale entry instead of handing it back. + SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + CountDownLatch pinned = new CountDownLatch(1); + CountDownLatch closed = new CountDownLatch(1); + AtomicReference secondCallError = new AtomicReference<>(); + Thread worker = new Thread(() -> { + try { + pool.pinToCurrentThread(); + pinned.countDown(); + Assert.assertTrue(closed.await(2, TimeUnit.SECONDS)); + try { + pool.pinToCurrentThread(); + secondCallError.set(new AssertionError("pinToCurrentThread after close must throw")); + } catch (LineSenderException e) { + // expected + } + } catch (Throwable t) { + secondCallError.set(t); + } + }); + worker.start(); + Assert.assertTrue(pinned.await(2, TimeUnit.SECONDS)); + pool.close(); + closed.countDown(); + worker.join(2_000); + if (secondCallError.get() != null) { + throw new AssertionError(secondCallError.get()); + } + } + + @Test + public void testReleaseAfterCloseIsSafe() throws Exception { + // Same setup as the pin test, but exercise releaseCurrentThread() + // instead. With a closed delegate underneath, the release path must + // not invoke flush() on the dead Sender. + SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + CountDownLatch pinned = new CountDownLatch(1); + CountDownLatch closed = new CountDownLatch(1); + AtomicReference releaseError = new AtomicReference<>(); + Thread worker = new Thread(() -> { + try { + pool.pinToCurrentThread(); + pinned.countDown(); + Assert.assertTrue(closed.await(2, TimeUnit.SECONDS)); + pool.releaseCurrentThread(); + } catch (Throwable t) { + releaseError.set(t); + } + }); + worker.start(); + Assert.assertTrue(pinned.await(2, TimeUnit.SECONDS)); + pool.close(); + closed.countDown(); + worker.join(2_000); + if (releaseError.get() != null) { + throw new AssertionError(releaseError.get()); + } + } + @Test public void testThreadAffinityIsPerThread() throws InterruptedException { try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 2, 2, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { From fd3f8bbb1f96efc26e82127b8a16a61f02979347 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 16:05:51 +0100 Subject: [PATCH 05/12] Add public drain(timeoutMillis) convenience method on Sender drain(timeoutMillis) flushes every buffered row and blocks until the server has acknowledged the resulting frame, or until the caller's timeout elapses. The shape mirrors the implicit drain that close() already runs -- target = flushAndGetSequence(), then awaitAckedFsn(target, timeoutMillis) -- but with the timeout chosen at the call site instead of at builder time via close_flush_timeout_millis. Useful for callers that want different drain budgets in different shutdown scenarios (long pre-batch checkpoint vs. tight per-iteration cleanup), or that want to observe drain progress without closing the sender. Pairs cleanly with close()'s implicit drain: drain(short) returns false on timeout without throwing, the caller can take an action (log, retry, switch to a different shutdown path), and a subsequent close() still runs its own bounded drain through close_flush_timeout_millis. Default implementation lives on the Sender interface so every transport picks it up: - WebSocket/QWP: target = real FSN; awaitAckedFsn blocks until acked. - HTTP / TCP / UDP (no FSN tracking): flushAndGetSequence returns -1, awaitAckedFsn(-1, _) returns true immediately. drain becomes "flush and report success" on these transports, which is the most honest thing a non-FSN transport can do. PooledSender forwards to its delegate so pool callers see the underlying Sender's semantics unchanged. Tests (CloseDrainTest): - testDrainBlocksUntilAckArrivesAndReturnsTrue: delayed-ACK server, drain(5000) must wait, return true; the subsequent close() is near-instant because the drain consumed the wait. - testDrainReturnsFalseOnTimeoutAndSenderStillUsable: silent server, drain(200) returns false (not throws). Sender continues to accept writes after. - testDrainNonZeroTimeoutOnFastServerReturnsImmediately: fast server, drain(5000) returns promptly -- the budget is an upper bound, not a floor. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../main/java/io/questdb/client/Sender.java | 23 +++++ .../io/questdb/client/impl/PooledSender.java | 5 ++ .../cutlass/qwp/client/CloseDrainTest.java | 89 +++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/core/src/main/java/io/questdb/client/Sender.java b/core/src/main/java/io/questdb/client/Sender.java index 5354a89c..f20d4311 100644 --- a/core/src/main/java/io/questdb/client/Sender.java +++ b/core/src/main/java/io/questdb/client/Sender.java @@ -466,6 +466,29 @@ default Sender decimalColumn(CharSequence name, CharSequence value) { */ Sender doubleColumn(CharSequence name, double value); + /** + * Convenience: flush every buffered row and block until the server has + * acknowledged the resulting frame, or until {@code timeoutMillis} elapses. + * Equivalent to {@code awaitAckedFsn(flushAndGetSequence(), timeoutMillis)}, + * which is the same shape as the implicit drain {@link #close()} runs -- + * with the caller controlling the timeout per call-site rather than + * relying on the builder-time {@code close_flush_timeout_millis}. + *
+ * Returns immediately on transports that do not track frame sequence + * numbers ({@code HTTP}, {@code TCP}, {@code UDP}): the flush still + * happens, the wait is a no-op, and the return value is {@code true}. + * + * @param timeoutMillis upper bound on the wait; {@code <= 0} returns the + * current state without blocking (the flush still + * happens before the check) + * @return {@code true} if the server has acknowledged every published + * frame on return, {@code false} on timeout + * @throws LineSenderException if the transport has latched a terminal error + */ + default boolean drain(long timeoutMillis) { + return awaitAckedFsn(flushAndGetSequence(), timeoutMillis); + } + /** * Add a column with a 32-bit floating point value. * diff --git a/core/src/main/java/io/questdb/client/impl/PooledSender.java b/core/src/main/java/io/questdb/client/impl/PooledSender.java index 3cf2ec98..19a8adad 100644 --- a/core/src/main/java/io/questdb/client/impl/PooledSender.java +++ b/core/src/main/java/io/questdb/client/impl/PooledSender.java @@ -214,6 +214,11 @@ public Sender doubleColumn(CharSequence name, double value) { return this; } + @Override + public boolean drain(long timeoutMillis) { + return delegate.drain(timeoutMillis); + } + @Override public Sender floatColumn(CharSequence name, float value) { delegate.floatColumn(name, value); diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java index 13168f60..0b2b3275 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java @@ -177,6 +177,95 @@ public void testCloseDrainTimesOutWhenAcksNeverArrive() throws Exception { } } + @Test + public void testDrainBlocksUntilAckArrivesAndReturnsTrue() throws Exception { + // Public drain(timeoutMillis): explicit pre-close drain that the + // caller controls per call-site. Same delayed-ACK server as + // testCloseBlocksUntilAckArrives, but the wait happens inside the + // explicit drain() call. The subsequent close() should be a near- + // instant no-op because everything is already acked. + int port = TestPorts.findUnusedPort(); + long ackDelayMs = 600; + DelayingAckHandler handler = new DelayingAckHandler(ackDelayMs); + try (TestWebSocketServer server = new TestWebSocketServer(port, handler)) { + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String cfg = "ws::addr=localhost:" + port + ";"; + try (Sender sender = Sender.fromConfig(cfg)) { + sender.table("foo").longColumn("v", 1L).atNow(); + long t0 = System.nanoTime(); + boolean drained = sender.drain(5_000); + long drainElapsedMs = (System.nanoTime() - t0) / 1_000_000; + Assert.assertTrue("drain(5000) must return true when the ACK arrives within budget", + drained); + Assert.assertTrue("drain returned too fast (no actual wait): " + drainElapsedMs + "ms", + drainElapsedMs >= ackDelayMs / 2); + + long c0 = System.nanoTime(); + sender.close(); + long closeElapsedMs = (System.nanoTime() - c0) / 1_000_000; + Assert.assertTrue("close() after drained sender should be near-instant, was " + + closeElapsedMs + "ms", + closeElapsedMs < ackDelayMs); + } + } + } + + @Test + public void testDrainReturnsFalseOnTimeoutAndSenderStillUsable() throws Exception { + // Server never ACKs. drain() with a small timeout must return false + // rather than throw (unlike close()'s implicit drain, which + // converts a timeout into a LineSenderException). The sender stays + // usable for further row writes after a false return; the + // outstanding frames remain pending and close()'s own drain still + // runs. + int port = TestPorts.findUnusedPort(); + SilentHandler handler = new SilentHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(port, handler)) { + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String cfg = "ws::addr=localhost:" + port + ";close_flush_timeout_millis=0;"; + try (Sender sender = Sender.fromConfig(cfg)) { + sender.table("foo").longColumn("v", 1L).atNow(); + long t0 = System.nanoTime(); + boolean drained = sender.drain(200); + long elapsedMs = (System.nanoTime() - t0) / 1_000_000; + Assert.assertFalse("drain must return false when the server never acks", drained); + Assert.assertTrue("drain returned far past the timeout: " + elapsedMs + "ms", + elapsedMs >= 150 && elapsedMs < 2_000); + // Sender must still be usable: write another row and flush + // without observing the latched error from the silent peer. + sender.table("foo").longColumn("v", 2L).atNow(); + sender.flush(); + } + } + } + + @Test + public void testDrainNonZeroTimeoutOnFastServerReturnsImmediately() throws Exception { + // Fast server: every frame is acked promptly. drain(longTimeout) + // must return true quickly -- no spurious wait when there is + // nothing to wait for. + int port = TestPorts.findUnusedPort(); + DelayingAckHandler handler = new DelayingAckHandler(0); + try (TestWebSocketServer server = new TestWebSocketServer(port, handler)) { + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String cfg = "ws::addr=localhost:" + port + ";"; + try (Sender sender = Sender.fromConfig(cfg)) { + sender.table("foo").longColumn("v", 1L).atNow(); + long t0 = System.nanoTime(); + Assert.assertTrue(sender.drain(5_000)); + long elapsedMs = (System.nanoTime() - t0) / 1_000_000; + Assert.assertTrue("drain on a fast server must return promptly, took " + elapsedMs + "ms", + elapsedMs < 2_000); + } + } + } + @Test public void testAsyncCloseDrainSucceedsWhenServerStartsDuringDrain() throws Exception { int port = TestPorts.findUnusedPort(); From 46673f877b9652ba40e05497125d20ae09b1318e Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 16:26:06 +0100 Subject: [PATCH 06/12] Raise default close_flush_timeout_millis from 5s to 60s The old 5s default was a "tight" budget that worked for benchmark-shape workloads (small batches, fast loopback, fresh server) but routinely fell short on real ones: slow consumers, catch-up replicas, chunky payloads against small server send buffers, GC pauses, network weather. When the budget runs out, close() throws and silently drops acked-but-unverified rows in memory mode -- which is a much worse failure mode than "close took a few seconds longer than I expected". The recent CI fragmentation work surfaced this concretely: a 25M-row async-mode test hit the close drain timeout because the server couldn't ACK the backlog within 5s; pinning the value to 60s gives the backlog enough wall-clock to drain even under the adversarial chunk sizes the fuzzer picks. 60s was chosen as the upper end of what feels "still bounded" without being open-ended -- humans typing Ctrl-C will tolerate up to about a minute of "shutdown in progress" before reaching for kill -9. The existing escape hatches stay in place: set close_flush_timeout_millis=0 or -1 for fast close (data lost in memory mode, recovered by the next sender in SF mode); set close_flush_timeout_millis to any positive value for a custom budget; or, new in fd3f8bb, call Sender.drain(timeoutMillis) explicitly before close() to choose the wait per call-site instead of per-builder. Test note: CloseDrainTest had two stale comments referring to the 5s default by name; updated to 60s. Behaviour-wise the tests still rely on "non-zero default, server takes ~800-1500 ms to ACK, close() must wait at least half that" -- a wider default does not invalidate any of those assertions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../main/java/io/questdb/client/Sender.java | 20 +++++++++++++++---- .../cutlass/qwp/client/CloseDrainTest.java | 6 +++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/io/questdb/client/Sender.java b/core/src/main/java/io/questdb/client/Sender.java index f20d4311..896113b0 100644 --- a/core/src/main/java/io/questdb/client/Sender.java +++ b/core/src/main/java/io/questdb/client/Sender.java @@ -925,10 +925,17 @@ final class LineSenderBuilder { private static final int DEFAULT_AUTO_FLUSH_INTERVAL_MILLIS = 1_000; private static final int DEFAULT_AUTO_FLUSH_ROWS = 75_000; private static final int DEFAULT_BUFFER_CAPACITY = 64 * 1024; - // Default close() drain timeout: block up to 5s waiting for the + // Default close() drain timeout: block up to 60s waiting for the // server to ACK everything published into the engine before - // shutting down the I/O loop. - private static final long DEFAULT_CLOSE_FLUSH_TIMEOUT_MILLIS = 5_000L; + // shutting down the I/O loop. The wide default reflects what real + // workloads need on the close path -- catch-up replicas, slow + // consumers, and small server send buffers under chunky payloads + // all routinely take tens of seconds to acknowledge a backlog, + // and silently dropping unacked rows in close() is a much worse + // default than spending the wall-clock to wait. Callers who want + // a tighter close budget either set close_flush_timeout_millis + // explicitly or call the new drain(timeoutMillis) before close(). + private static final long DEFAULT_CLOSE_FLUSH_TIMEOUT_MILLIS = 60_000L; private static final int DEFAULT_HTTP_PORT = 9000; private static final int DEFAULT_HTTP_TIMEOUT = 30_000; private static final int DEFAULT_MAXIMUM_BUFFER_CAPACITY = 100 * 1024 * 1024; @@ -1627,7 +1634,12 @@ public Sender build() { * close() drain timeout in milliseconds. The sender's {@code close()} * method blocks up to this many millis waiting for the server to ACK * every batch already published into the engine before shutting down - * the I/O loop. Default {@code 5000}. + * the I/O loop. Default {@code 60000} (60 s) -- generous enough to + * survive real-workload backlogs (slow consumers, catch-up replicas, + * chunky payloads on small server send buffers) without silently + * dropping unacked rows; callers that need a longer pre-close wait + * for a specific submission can call + * {@link Sender#drain(long)} explicitly before close(). *

* Set to {@code 0} or {@code -1} to opt out — close() will not wait * at all (fast close). Pending data is then lost in memory mode and diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java index 0b2b3275..d836b9c0 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java @@ -52,8 +52,8 @@ public class CloseDrainTest { @Test public void testCloseBlocksUntilAckArrives() throws Exception { // Server delays every ACK by 800ms. With the default - // close_flush_timeout_millis=5000, close() must wait for that ACK - // before returning. Pre-fix close() returned within milliseconds. + // close_flush_timeout_millis=60000, close() must wait for that + // ACK before returning. Pre-fix close() returned within milliseconds. int port = TestPorts.findUnusedPort(); long ackDelayMs = 800; DelayingAckHandler handler = new DelayingAckHandler(ackDelayMs); @@ -113,7 +113,7 @@ public void testCloseFastWhenTimeoutIsMinusOne() throws Exception { // // Currently fails because -1 collides with the PARAMETER_NOT_SET_EXPLICITLY // sentinel in LineSenderBuilder, so the build path silently substitutes - // DEFAULT_CLOSE_FLUSH_TIMEOUT_MILLIS (5000ms) and close() blocks for the + // DEFAULT_CLOSE_FLUSH_TIMEOUT_MILLIS (60s) and close() blocks for the // full ACK delay instead of returning fast. int port = TestPorts.findUnusedPort(); long ackDelayMs = 1500; From 3376db66f1983d55593388dd12f0b40ea7f5ed19 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Thu, 21 May 2026 18:31:14 +0100 Subject: [PATCH 07/12] Stop SenderPool.close from racing discardBroken on `all` SenderPool.close()'s teardown loop iterated `all` outside the ReentrantLock with the comment "Snapshot of underlying Senders to close" -- but the code iterated the live ArrayList, not a snapshot. Concurrently, discardBroken acquired the lock and called all.remove(s) regardless of whether the pool was already shutting down. ArrayList is not thread-safe; close()'s for (int i = 0; i < all.size(); i++) loop reads `size` and indexed elements without synchronisation while discardBroken's fastRemove updates size and shifts elements down. Reachable interleaving: 1. Application calls db.close() -> pool.close() on thread A. 2. Thread B is in a try-with-resources block holding a borrowed PooledSender. The block exits, PooledSender.close() runs. 3. A acquires the lock, sets closed=true, marks every wrapper invalidated, releases the lock, starts the teardown loop, closes B's delegate. 4. B's delegate.flush() throws (closed socket); broken=true routes to discardBroken. 5. discardBroken acquires the lock (A no longer holds it), removes B's wrapper from `all`, releases the lock, calls delegate.close() on B's already-closed delegate. Outcomes range from IndexOutOfBoundsException out of close() (propagating up the shutdown path), through native-handle leaks when the iteration cursor and the remove shift miss each other, through two threads simultaneously inside delegate.close() on the same Sender (whose teardown frees native scratch and joins a daemon -- not safe to enter from concurrent threads). Fix has two parts: - close() takes a snapshot under the lock (PooledSender[] snapshot = all.toArray(...)) and iterates the snapshot outside the lock. The teardown loop is now immune to concurrent mutation of `all`, regardless of whether mutators cooperate. - discardBroken bails on `closed`. Once the pool is shutting down, close()'s snapshot loop owns the delegate close; mutating `all` here would race that iteration (with the snapshot in place that is no longer a correctness issue, but it is still wasted work) and the delegate.close() below would be a double-close. Add an if (closed) return; immediately under the lock; the read is serialised against close()'s closed=true write because both happen under `lock`. Red test (testDiscardBrokenAfterCloseDoesNotMutatePool) drives the scenario deterministically on a single thread: borrow a sender, write a row so flush has something to send, pool.close() (which closes the delegate), then sender.close() (whose flush throws, broken=true, route to discardBroken). Before the fix pool.totalSize() drops from 1 to 0 because discardBroken removed the wrapper from `all` post-close; after the fix it stays at 1. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../io/questdb/client/impl/SenderPool.java | 25 +++++++-- .../client/test/impl/SenderPoolTest.java | 54 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/io/questdb/client/impl/SenderPool.java b/core/src/main/java/io/questdb/client/impl/SenderPool.java index c463f948..6082b8bd 100644 --- a/core/src/main/java/io/questdb/client/impl/SenderPool.java +++ b/core/src/main/java/io/questdb/client/impl/SenderPool.java @@ -173,6 +173,7 @@ public PooledSender borrow() { @Override public void close() { + PooledSender[] snapshot; lock.lock(); try { if (closed) { @@ -187,16 +188,23 @@ public void close() { for (int i = 0; i < all.size(); i++) { all.get(i).markInvalidated(); } + // Snapshot under the lock so the delegate-close loop below is + // immune to concurrent mutation of `all`. discardBroken running + // on another thread can still bail thanks to the `closed` check + // it now performs; the snapshot is belt-and-braces for any + // future code path that mutates `all` outside this lock's + // happens-before chain. + snapshot = all.toArray(new PooledSender[0]); threadAffine.remove(); slotReleased.signalAll(); } finally { lock.unlock(); } - // Snapshot of underlying Senders to close, taken outside the lock so - // a slow real-close() doesn't keep the pool latched. - for (int i = 0; i < all.size(); i++) { + // Close each delegate from the snapshot, outside the lock so a slow + // real-close() doesn't keep the pool latched. + for (int i = 0; i < snapshot.length; i++) { try { - all.get(i).delegate().close(); + snapshot[i].delegate().close(); } catch (RuntimeException ignored) { } } @@ -210,11 +218,20 @@ public void close() { * {@code all} so the pool can grow back into a fresh slot on demand. The * underlying delegate is closed outside the lock so a slow real-close * does not stall other borrowers. + *

+ * Bails when the pool is already closed: {@link #close()} owns the + * teardown of every delegate via its snapshot loop, so mutating + * {@code all} here would race that iteration on a non-thread-safe + * {@code ArrayList} and the {@code delegate.close()} below would be a + * double-close on a delegate {@code close()} has already shut down. */ void discardBroken(PooledSender s) { s.markInvalidated(); lock.lock(); try { + if (closed) { + return; + } all.remove(s); // Wake one waiter -- the cap check in borrow() uses all.size(), // so a freed slot may now allow a creation attempt. diff --git a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java index a5b043a3..1df684d7 100644 --- a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java +++ b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java @@ -98,6 +98,60 @@ public void testCloseIdempotent() { pool.close(); } + @Test + public void testDiscardBrokenAfterCloseDoesNotMutatePool() { + // Race: pool.close() iterates `all` outside the lock to close each + // delegate. Concurrently, a borrower's PooledSender.close() sees its + // delegate already closed (closed by pool.close()), the flush throws, + // broken=true routes to SenderPool.discardBroken -- which previously + // called all.remove(s) and delegate.close() unconditionally, + // racing the iteration in pool.close() on a non-thread-safe + // ArrayList. Possible outcomes: IndexOutOfBoundsException out of + // pool.close(), skipped delegate close (native handle leak), or + // two threads simultaneously inside delegate.close(). + // + // The fix gates discardBroken on `closed`: once the pool is shutting + // down, close()'s teardown loop owns the delegate close and + // discardBroken bails before touching `all`. + // + // Deterministic reproduction: serialise the race onto one thread by + // calling pool.close() first (which closes the delegate of every + // pooled wrapper), then driving sender.close() second. Without the + // fix, sender.close() routes to discardBroken, which removes the + // wrapper from `all` post-close -- visible as totalSize dropping + // from 1 to 0. With the fix, discardBroken sees closed=true and + // bails, leaving `all` untouched. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + Sender s = pool.borrow(); + // A row in the buffer forces flush() to attempt a real HTTP + // request on close(); against the unreachable DEAD_HTTP target + // (and now also against the closed delegate below) flush throws + // and routes the wrapper to discardBroken. + s.table("t").longColumn("v", 1L).atNow(); + + pool.close(); + Assert.assertEquals( + "pool.close() must not clear `all` -- the teardown loop closes delegates in place", + 1, pool.totalSize() + ); + + // sender.close() now hits a closed delegate; flush throws; the + // PooledSender.close() finally routes to discardBroken. + try { + s.close(); + } catch (LineSenderException expected) { + // expected: flush against a closed delegate throws. + } catch (RuntimeException expected) { + // some Sender implementations wrap differently; either is fine. + } + + Assert.assertEquals( + "discardBroken called after pool close must NOT mutate `all`", + 1, pool.totalSize() + ); + } + } + @Test public void testCloseRejectsSubsequentBorrow() { SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); From 41a0d98e8edcfe9c974886cc89499c722e9a101b Mon Sep 17 00:00:00 2001 From: bluestreak Date: Fri, 22 May 2026 13:55:05 +0100 Subject: [PATCH 08/12] Free QwpQueryClient when QueryClientPool connect fails QueryClientPool.createUnlocked() constructs a QwpQueryClient via QwpQueryClient.fromConfig() and then calls client.connect(). The construction step field-initialises QwpBindValues, which allocates 8192 NATIVE_DEFAULT bytes through NativeBufferWriter. When connect() throws -- server unreachable, role mismatch (target=primary against a REPLICA), TLS handshake error, etc. -- the client reference escapes: the catch in acquire() (lines 126-133) only has handles to inFlightCreations and the lock, and the pre-warm cleanup in the constructor (lines 94-102) iterates `for (i < built)` over `all`, which never received the failed worker. Every connect failure during pool growth leaks one bindValues buffer; a flapping endpoint accumulates the leak per attempt. createUnlocked() now catches RuntimeException from connect(), calls client.close(), and rethrows. QwpQueryClient.close() guards against pre-connect state via closedFlag and runs bindValues.close() in a finally block, so closing a never-connected client releases the NativeBufferWriter scratch without touching the still-null ioThread or webSocketClient. QueryClientPoolLeakTest covers both the pre-warm and lazy-create paths. It points a QwpQueryClient at a FakeStatusServer that returns HTTP 421 and X-QuestDB-Role: REPLICA while the client requests target=primary -- the endpoint walk rejects the only option and connect() throws HttpClientException deterministically. The test asserts that Unsafe.getMemUsedByTag(NATIVE_DEFAULT) returns to the pre-failure baseline. Before the fix both tests fail with an 8192-byte delta; after the fix the delta is zero. The QueryClientPool constructor and acquire() become public to match SenderPool's testability surface (its constructor and borrow() are already public). QueryWorker was already public. --- .../questdb/client/impl/QueryClientPool.java | 19 +- .../test/impl/QueryClientPoolLeakTest.java | 179 ++++++++++++++++++ 2 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 core/src/test/java/io/questdb/client/test/impl/QueryClientPoolLeakTest.java diff --git a/core/src/main/java/io/questdb/client/impl/QueryClientPool.java b/core/src/main/java/io/questdb/client/impl/QueryClientPool.java index 56d5ec7e..8191f162 100644 --- a/core/src/main/java/io/questdb/client/impl/QueryClientPool.java +++ b/core/src/main/java/io/questdb/client/impl/QueryClientPool.java @@ -62,7 +62,7 @@ public final class QueryClientPool { private volatile boolean closed; private int inFlightCreations; - QueryClientPool( + public QueryClientPool( String configurationString, int minSize, int maxSize, @@ -102,7 +102,7 @@ public final class QueryClientPool { } } - QueryWorker acquire() { + public QueryWorker acquire() { // Track remaining wait via awaitNanos's return value (canonical pattern): // awaitNanos consumes from the budget on each wait and reports what is // left; <= 0 means the budget is exhausted. @@ -241,7 +241,20 @@ void release(QueryWorker w) { private QueryWorker createUnlocked() { QwpQueryClient client = QwpQueryClient.fromConfig(configurationString); - client.connect(); + try { + client.connect(); + } catch (RuntimeException e) { + // connect() may throw after QwpQueryClient.fromConfig() has already + // allocated native scratch (the QwpBindValues NativeBufferWriter is + // field-initialised). Close the half-built client so its allocations + // are released, otherwise every connect failure during pool growth + // leaks NATIVE_DEFAULT bytes. + try { + client.close(); + } catch (RuntimeException ignored) { + } + throw e; + } return new QueryWorker(client, this, nextSlotIndex.getAndIncrement()); } } diff --git a/core/src/test/java/io/questdb/client/test/impl/QueryClientPoolLeakTest.java b/core/src/test/java/io/questdb/client/test/impl/QueryClientPoolLeakTest.java new file mode 100644 index 00000000..53def4b5 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/impl/QueryClientPoolLeakTest.java @@ -0,0 +1,179 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.impl; + +import io.questdb.client.impl.QueryClientPool; +import io.questdb.client.std.MemoryTag; +import io.questdb.client.std.Unsafe; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.atomic.AtomicInteger; + +public class QueryClientPoolLeakTest { + + // QwpBindValues holds a NativeBufferWriter whose default buffer is + // 8192 bytes tagged NATIVE_DEFAULT. It is allocated at QwpQueryClient + // field-init time, which means QwpQueryClient.fromConfig() commits this + // allocation before connect() is ever called. + // + // Both of these tests construct a scenario where connect() reliably throws: + // a FakeStatusServer returns HTTP 421 with X-QuestDB-Role: REPLICA, and the + // client requests target=primary. The walk rejects the only endpoint and + // connect() throws HttpClientException. + // + // If QueryClientPool.createUnlocked() does not close the half-built client + // when connect() throws, the NATIVE_DEFAULT counter ends higher than it + // started. The assertion compares before/after directly: no leak means + // delta == 0. + + @Test(timeout = 10_000) + public void acquireDoesNotLeakNativeScratchOnConnectFailure() throws Exception { + try (FakeStatusServer rejecter = new FakeStatusServer(421, "X-QuestDB-Role: REPLICA")) { + rejecter.start(); + String cfg = "ws::addr=127.0.0.1:" + rejecter.port() + + ";target=primary;failover=off;auth_timeout_ms=1000;"; + + QueryClientPool pool = new QueryClientPool( + cfg, 0, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + try { + long baseline = Unsafe.getMemUsedByTag(MemoryTag.NATIVE_DEFAULT); + try { + pool.acquire(); + Assert.fail("expected acquire() to throw on connect rejection"); + } catch (RuntimeException expected) { + // QueryException wrapping the underlying connect failure. + } + long after = Unsafe.getMemUsedByTag(MemoryTag.NATIVE_DEFAULT); + Assert.assertEquals( + "acquire() leaked NATIVE_DEFAULT bytes on connect failure", + baseline, after); + } finally { + pool.close(); + } + } + } + + @Test(timeout = 10_000) + public void preWarmDoesNotLeakNativeScratchOnConnectFailure() throws Exception { + try (FakeStatusServer rejecter = new FakeStatusServer(421, "X-QuestDB-Role: REPLICA")) { + rejecter.start(); + String cfg = "ws::addr=127.0.0.1:" + rejecter.port() + + ";target=primary;failover=off;auth_timeout_ms=1000;"; + + long baseline = Unsafe.getMemUsedByTag(MemoryTag.NATIVE_DEFAULT); + try { + new QueryClientPool(cfg, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE); + Assert.fail("expected QueryClientPool ctor to throw on connect rejection"); + } catch (RuntimeException expected) { + // target=primary against role=REPLICA yields a connect failure + // out of createUnlocked(). + } + long after = Unsafe.getMemUsedByTag(MemoryTag.NATIVE_DEFAULT); + Assert.assertEquals( + "pool ctor leaked NATIVE_DEFAULT bytes on connect failure", + baseline, after); + } + } + + private static final class FakeStatusServer implements AutoCloseable { + final AtomicInteger connections = new AtomicInteger(); + private final String roleHeader; + private final ServerSocket socket; + private final int statusCode; + private volatile boolean running = true; + + FakeStatusServer(int statusCode, String roleHeader) throws IOException { + this.statusCode = statusCode; + this.roleHeader = roleHeader; + this.socket = new ServerSocket(0, 50, InetAddress.getLoopbackAddress()); + } + + @Override + public void close() throws IOException { + running = false; + socket.close(); + } + + int port() { + return socket.getLocalPort(); + } + + void start() { + Thread t = new Thread(this::loop, "fake-status-" + statusCode); + t.setDaemon(true); + t.start(); + } + + private void handle(Socket s) { + try (Socket sock = s) { + connections.incrementAndGet(); + byte[] discard = new byte[8192]; + int n = sock.getInputStream().read(discard); + if (n < 0) return; + StringBuilder resp = new StringBuilder(); + resp.append("HTTP/1.1 ").append(statusCode).append(' ').append(reason(statusCode)).append("\r\n"); + if (roleHeader != null) { + resp.append(roleHeader).append("\r\n"); + } + resp.append("Content-Length: 0\r\nConnection: close\r\n\r\n"); + OutputStream out = sock.getOutputStream(); + out.write(resp.toString().getBytes(StandardCharsets.US_ASCII)); + out.flush(); + } catch (Exception ignored) { + } + } + + private void loop() { + while (running) { + try { + Socket s = socket.accept(); + Thread h = new Thread(() -> handle(s), "fake-status-handler-" + statusCode); + h.setDaemon(true); + h.start(); + } catch (IOException e) { + if (!running) return; + } + } + } + + private static String reason(int code) { + switch (code) { + case 401: + return "Unauthorized"; + case 421: + return "Misdirected Request"; + default: + return "Status"; + } + } + } +} From d4a3987a1573ade1025390d1667f3ba8c91c3d8f Mon Sep 17 00:00:00 2001 From: bluestreak Date: Fri, 22 May 2026 13:54:51 +0100 Subject: [PATCH 09/12] Fix pinToCurrentThread sharing after user close pinToCurrentThread() caches the borrowed PooledSender in a ThreadLocal (threadAffine) so subsequent db.sender() calls on the same thread skip the borrow path. PooledSender.close() -- the public Sender#close() the user reaches for in try (Sender s = db.sender()) { ... } -- flushed and returned the wrapper to the pool, but never cleared the caller's ThreadLocal entry. A subsequent db.sender() on the same thread would re-read the cached entry, see isInvalidated() = false, and return the wrapper even though another consumer had since borrowed the slot. Both consumers then wrote to the same underlying Sender; ILP rows interleaved on the wire. Reachable interleaving: 1. Thread A: db.sender() -> S, inUse=true, A's TL=S. 2. Thread A: S.close() -> inUse=false, S returned to pool; A's TL still references S. 3. Thread B: db.sender() -> borrow() polls S from `available`, markInUse=true, returns S to B. 4. Thread A: db.sender() -> TL.get() = S, !invalidated -> returns S. A and B now share S. The earlier defensive idea of "also check pinned.isInUse() in pinToCurrentThread()" only narrows the window: it catches the case where A re-pins before B borrows, but once B's borrow has set inUse=true the guard passes and the bug repeats. Fix is structural: clear the pin in PooledSender.close() before the slot becomes borrowable again. - New package-private SenderPool.clearPinIfCurrent(s) removes the current thread's TL entry iff it currently references s. - PooledSender.close() calls it in the finally block, BEFORE pool.giveBack(this) / pool.discardBroken(this). The order matters: if we cleared after giveBack, a concurrent borrower could grab the slot while the TL still pointed at the wrapper, and a re-pin on this thread would hand the (now in-use) wrapper back -- the same race this clear is meant to close. clearPinIfCurrent is also called on the discardBroken path. There it is redundant -- markInvalidated() means the existing isInvalidated() check in pinToCurrentThread() would already clear the TL on the next pin -- but uniformly clearing is simpler and makes the close() contract symmetric across the broken/healthy branches. Behaviour change: db.sender() after s.close() on the same thread no longer returns the same wrapper instance. It goes through borrow() again and may return a different slot, or block on an empty pool. This is the correct semantics -- close() releases the pin; releaseSender() remains the documented release path for code that wants to keep using the same wrapper without flushing. Red tests (in SenderPoolTest): - testPinAfterUserCloseDoesNotShareWrapper: same-thread reproducer, pool size 1 so the in-between borrow deterministically receives the just-returned slot. Bug exposed at the wrapper-identity level without timing. Before fix, pinToCurrentThread() returns the (now in-use) wrapper; after fix, it tries to borrow from an empty pool and trips the 100ms acquireTimeout, throwing LineSenderException. - testPinAfterUserCloseDoesNotShareWrapperCrossThread: mirrors the originally reported A/B scenario with two threads sequenced via CountDownLatch. Same identity assertion. Same green/red flip across the fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../io/questdb/client/impl/PooledSender.java | 15 ++++ .../io/questdb/client/impl/SenderPool.java | 14 +++ .../client/test/impl/SenderPoolTest.java | 85 +++++++++++++++++++ 3 files changed, 114 insertions(+) diff --git a/core/src/main/java/io/questdb/client/impl/PooledSender.java b/core/src/main/java/io/questdb/client/impl/PooledSender.java index 19a8adad..9e2dbbb6 100644 --- a/core/src/main/java/io/questdb/client/impl/PooledSender.java +++ b/core/src/main/java/io/questdb/client/impl/PooledSender.java @@ -133,6 +133,15 @@ public Sender charColumn(CharSequence name, char value) { * the owning {@code QuestDB} is closed. *

* Idempotent: a second call after a return is a no-op. + *

+ * Clears the current thread's pin (if any) before the slot becomes + * borrowable again. Without this step a thread that pinned this + * wrapper and then closed it via the public {@link Sender#close()} + * (the natural try-with-resources idiom) would still hold the pin + * in its {@link ThreadLocal}; a subsequent {@code QuestDB.sender()} + * call on that thread would return the cached wrapper even though + * another consumer has since borrowed the slot, and the two + * consumers would write to the same underlying delegate. */ @Override public void close() { @@ -152,6 +161,12 @@ public void close() { throw e; } finally { inUse = false; + // Clear the pin BEFORE returning the slot. If we cleared + // after giveBack(), a concurrent borrower could grab the + // slot while this thread's pin still references it, and a + // re-pin on this thread would return the (now in-use) + // wrapper -- the same race this clear is meant to close. + pool.clearPinIfCurrent(this); if (broken) { pool.discardBroken(this); } else { diff --git a/core/src/main/java/io/questdb/client/impl/SenderPool.java b/core/src/main/java/io/questdb/client/impl/SenderPool.java index 6082b8bd..61b6ac69 100644 --- a/core/src/main/java/io/questdb/client/impl/SenderPool.java +++ b/core/src/main/java/io/questdb/client/impl/SenderPool.java @@ -210,6 +210,20 @@ public void close() { } } + /** + * Clears the current thread's pin if it currently references {@code s}. + * Invoked from {@link PooledSender#close()} before the wrapper is + * returned to the pool, so a subsequent {@link #pinToCurrentThread()} + * on this thread cannot hand the wrapper back after another consumer + * has borrowed the slot. No-op when the caller never pinned, or pinned + * a different wrapper. + */ + void clearPinIfCurrent(PooledSender s) { + if (threadAffine.get() == s) { + threadAffine.remove(); + } + } + /** * Evicts a slot whose delegate has failed (typically a {@code flush()} * failure observed in {@link PooledSender#close()}). The wrapper is diff --git a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java index 1df684d7..7c58943a 100644 --- a/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java +++ b/core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java @@ -349,6 +349,91 @@ public void testPinAfterCloseRejectsStaleEntry() throws Exception { } } + @Test + public void testPinAfterUserCloseDoesNotShareWrapper() { + // Same-thread reproducer for the pinToCurrentThread() sharing bug. + // The user closes a pinned Sender (the natural try-with-resources + // idiom on the public Sender API), then another consumer borrows + // the slot. pinToCurrentThread() must not hand that wrapper back: + // it is now owned by the second consumer. + // + // Pool size 1 collapses the race window into a linear sequence: + // the second borrower deterministically receives the same slot + // that was just returned, so the bug is observable at the + // wrapper-identity level without timing. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 100, Long.MAX_VALUE, Long.MAX_VALUE)) { + Sender pinned = pool.pinToCurrentThread(); + pinned.close(); // pool slot returned; ThreadLocal still points at it + Sender stolen = pool.borrow(); // pollFirst hands the same wrapper to a new consumer + try { + Sender repinned = pool.pinToCurrentThread(); + Assert.fail("pinToCurrentThread() returned wrapper " + repinned + + " already borrowed by another consumer " + stolen); + } catch (LineSenderException expected) { + // After fix: TL cleared (or owner-thread invalidated) on close; + // re-pin tries to borrow, pool is empty, acquireTimeout fires. + } finally { + stolen.close(); + } + } + } + + @Test + public void testPinAfterUserCloseDoesNotShareWrapperCrossThread() throws InterruptedException { + // Cross-thread variant of the same bug, mirroring the originally + // reported trigger: Thread A pins, closes, then re-pins while + // Thread B has borrowed the slot in between. A's ThreadLocal still + // references the wrapper, and pinToCurrentThread() hands it back -- + // so A and B end up writing to the same underlying Sender. + try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 1, 1, 100, Long.MAX_VALUE, Long.MAX_VALUE)) { + CountDownLatch aClosed = new CountDownLatch(1); + CountDownLatch bBorrowed = new CountDownLatch(1); + AtomicReference bSender = new AtomicReference<>(); + AtomicReference failure = new AtomicReference<>(); + + Thread a = new Thread(() -> { + try { + Sender s = pool.pinToCurrentThread(); + s.close(); + aClosed.countDown(); + Assert.assertTrue(bBorrowed.await(2, TimeUnit.SECONDS)); + try { + Sender repinned = pool.pinToCurrentThread(); + failure.compareAndSet(null, new AssertionError( + "pinToCurrentThread() returned wrapper " + repinned + + " already borrowed by another thread " + bSender.get())); + } catch (LineSenderException expected) { + // After fix: re-pin tries to borrow, pool is empty, times out. + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } + }); + Thread b = new Thread(() -> { + try { + Assert.assertTrue(aClosed.await(2, TimeUnit.SECONDS)); + bSender.set(pool.borrow()); + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + bBorrowed.countDown(); + } + }); + + a.start(); + b.start(); + a.join(4_000); + b.join(4_000); + + if (bSender.get() != null) { + bSender.get().close(); + } + if (failure.get() != null) { + throw new AssertionError(failure.get()); + } + } + } + @Test public void testReleaseAfterCloseIsSafe() throws Exception { // Same setup as the pin test, but exercise releaseCurrentThread() From 558ad5f94c6b783e1596dc369dbe534287743c99 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Fri, 22 May 2026 14:19:07 +0100 Subject: [PATCH 10/12] review fix --- .../io/questdb/client/impl/QueryWorker.java | 17 +++ .../cutlass/qwp/client/ReconnectTest.java | 58 +++------ .../test/cutlass/qwp/client/TestPorts.java | 26 +--- .../qwp/websocket/TestWebSocketServer.java | 28 ++--- .../client/test/impl/QueryWorkerTest.java | 115 ++++++++++++++++++ 5 files changed, 156 insertions(+), 88 deletions(-) diff --git a/core/src/main/java/io/questdb/client/impl/QueryWorker.java b/core/src/main/java/io/questdb/client/impl/QueryWorker.java index b8e2ccd6..4b251431 100644 --- a/core/src/main/java/io/questdb/client/impl/QueryWorker.java +++ b/core/src/main/java/io/questdb/client/impl/QueryWorker.java @@ -24,6 +24,7 @@ package io.questdb.client.impl; +import io.questdb.client.QueryException; import io.questdb.client.cutlass.qwp.client.QwpQueryClient; import java.util.concurrent.locks.Condition; @@ -133,6 +134,13 @@ void start() { void dispatch(QueryImpl q) { signalLock.lock(); try { + if (shuttingDown) { + // shutdown() has already flipped the flag and signalled the + // worker thread, so the run loop will not pick this up. Signal + // the caller directly so its Completion.await() does not hang. + q.signalUnexpected(new QueryException((byte) 0, "QuestDB handle is closed")); + return; + } current = q; signalCondition.signal(); } finally { @@ -149,6 +157,15 @@ private void runLoop() { signalCondition.awaitUninterruptibly(); } if (shuttingDown) { + // shutdown() raced an in-flight dispatch(): current was set + // by a caller but the loop never got to runOn(). Signal the + // caller so its Completion.await() does not hang. + QueryImpl stranded = current; + current = null; + if (stranded != null) { + stranded.signalUnexpected( + new QueryException((byte) 0, "QuestDB handle is closed")); + } return; } q = current; diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/ReconnectTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/ReconnectTest.java index cb2f34df..1e9d99d1 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/ReconnectTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/ReconnectTest.java @@ -101,15 +101,14 @@ public void testReconnectAfterServerInducedDisconnect() throws Exception { } @Test - public void testReconnectGivesUpAfterCap() throws Exception { + public void testReconnectGivesUpAfterCap() { // Server is up at first (initial connect succeeds + ACKs batch 1), // then we tear it down — subsequent reconnect attempts get TCP // connection-refused and accumulate against the budget. With a // 500ms cap, the loop should give up well inside the test's 5s // poll window and the next user-thread flush() must throw. int port = TestPorts.findUnusedPort(); - TestWebSocketServer server = new TestWebSocketServer(port, new AckHandler()); - try { + try (TestWebSocketServer server = new TestWebSocketServer(port, new AckHandler())) { server.start(); Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); @@ -118,8 +117,7 @@ public void testReconnectGivesUpAfterCap() throws Exception { + ";reconnect_initial_backoff_millis=10" + ";reconnect_max_backoff_millis=50" + ";close_flush_timeout_millis=0;"; - Sender sender = Sender.fromConfig(cfg); - try { + try (Sender sender = Sender.fromConfig(cfg)) { sender.table("foo").longColumn("v", 1L).atNow(); sender.flush(); @@ -131,7 +129,7 @@ public void testReconnectGivesUpAfterCap() throws Exception { Throwable observed = null; long deadline = System.currentTimeMillis() + 5_000; long iter = 0; - while (System.currentTimeMillis() < deadline && observed == null) { + while (System.currentTimeMillis() < deadline) { iter++; try { sender.table("foo").longColumn("v", iter).atNow(); @@ -151,20 +149,12 @@ public void testReconnectGivesUpAfterCap() throws Exception { msg.contains("reconnect failed") || msg.contains("I/O thread failed") || msg.contains("Failed to connect")); - } finally { - // close() rethrows the latched terminal reconnect-cap error - // (commit 052f6ee). Already observed and asserted above. - try { - sender.close(); - } catch (LineSenderException ignored) { - } - } - } finally { - try { - server.close(); - } catch (Exception ignored) { - // already closed + } catch (LineSenderException ignored) { } + // close() rethrows the latched terminal reconnect-cap error + // (commit 052f6ee). Already observed and asserted above. + } catch (Exception ignored) { + // already closed } } @@ -184,8 +174,7 @@ public void testTerminalUpgradeErrorAbortsReconnect() throws Exception { String cfg = "ws::addr=localhost:" + port + ";reconnect_max_duration_millis=10000" + ";close_flush_timeout_millis=0;"; - Sender sender = Sender.fromConfig(cfg); - try { + try (Sender sender = Sender.fromConfig(cfg)) { sender.table("foo").longColumn("v", 1L).atNow(); sender.flush(); // Wait for first connection to ACK + close @@ -194,7 +183,7 @@ public void testTerminalUpgradeErrorAbortsReconnect() throws Exception { long t0 = System.nanoTime(); Throwable observed = null; long deadline = System.currentTimeMillis() + 5_000; - while (System.currentTimeMillis() < deadline && observed == null) { + while (System.currentTimeMillis() < deadline) { try { sender.table("foo").longColumn("v", 2L).atNow(); sender.flush(); @@ -217,14 +206,10 @@ public void testTerminalUpgradeErrorAbortsReconnect() throws Exception { msg.contains("WebSocket upgrade failed") || msg.contains("I/O thread failed") || msg.contains("401")); - } finally { - // close() rethrows the latched terminal upgrade error - // (commit 052f6ee). Already observed and asserted above. - try { - sender.close(); - } catch (LineSenderException ignored) { - } + } catch (LineSenderException ignored) { } + // close() rethrows the latched terminal upgrade error + // (commit 052f6ee). Already observed and asserted above. } } @@ -427,7 +412,7 @@ private void handleClient(Socket s, boolean firstConnection) { BufferedReader in = new BufferedReader(new InputStreamReader( s.getInputStream(), StandardCharsets.US_ASCII)); OutputStream out = s.getOutputStream(); - String requestLine = in.readLine(); + in.readLine(); String secKey = null; String line; while ((line = in.readLine()) != null && !line.isEmpty()) { @@ -535,19 +520,6 @@ public void close() { } } - /** Closes every connection right after receiving the first frame. */ - private static class AlwaysDisconnectHandler implements TestWebSocketServer.WebSocketServerHandler { - @Override - public void onBinaryMessage(TestWebSocketServer.ClientHandler client, byte[] data) { - try { - Thread.sleep(10); - client.close(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } - /** Acks every binary frame so the sender doesn't hang. */ private static class AckHandler implements TestWebSocketServer.WebSocketServerHandler { private final AtomicLong nextSeq = new AtomicLong(0); diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/TestPorts.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/TestPorts.java index 0f384817..43b3e8e0 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/TestPorts.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/TestPorts.java @@ -25,6 +25,7 @@ package io.questdb.client.test.cutlass.qwp.client; import java.io.IOException; +import java.net.InetAddress; import java.net.ServerSocket; public final class TestPorts { @@ -33,33 +34,10 @@ private TestPorts() { } public static int findUnusedPort() { - try (ServerSocket s = new ServerSocket(0)) { + try (ServerSocket s = new ServerSocket(0, 50, InetAddress.getLoopbackAddress())) { return s.getLocalPort(); } catch (IOException e) { throw new RuntimeException("failed to allocate an ephemeral port", e); } } - - public static int[] findUnusedPorts(int n) { - ServerSocket[] sockets = new ServerSocket[n]; - int[] ports = new int[n]; - try { - for (int i = 0; i < n; i++) { - sockets[i] = new ServerSocket(0); - ports[i] = sockets[i].getLocalPort(); - } - } catch (IOException e) { - throw new RuntimeException("failed to allocate ephemeral ports", e); - } finally { - for (ServerSocket s : sockets) { - if (s != null) { - try { - s.close(); - } catch (IOException ignore) { - } - } - } - } - return ports; - } } diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java index a50300dc..3da4f829 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java @@ -61,10 +61,6 @@ public class TestWebSocketServer implements Closeable { private final AtomicBoolean running = new AtomicBoolean(false); private final CountDownLatch startLatch = new CountDownLatch(1); private Thread acceptThread; - // X-QWP-Max-Batch-Size value to emit on the 101 handshake response. - // 0 = omit the header (legacy behavior). Tests that exercise the - // batch-size-clamp path on the client set this to a positive value. - private volatile int advertisedMaxBatchSize; // X-QuestDB-Role value to emit on handshake responses. null = omit the // header (legacy behavior for tests written before role-aware failover). // The server emits the header on both the 101 success path and (when @@ -148,14 +144,6 @@ public void close() { } } - /** - * Sets the X-QWP-Max-Batch-Size header value emitted on subsequent - * handshake responses. 0 (the default) omits the header. - */ - public void setAdvertisedMaxBatchSize(int maxBatchSize) { - this.advertisedMaxBatchSize = maxBatchSize; - } - /** * Replaces the advertised role for subsequent handshakes (live update). */ @@ -190,7 +178,13 @@ public void start() throws IOException { return; } - serverSocket = new ServerSocket(port); + // Bind explicitly to the loopback address. The wildcard 0.0.0.0 + // default lets a leftover process holding 127.0.0.1:port coexist + // on the same port under BSD/macOS SO_REUSEADDR semantics, and + // client connections to "localhost" then route to the more-specific + // listener instead of this mock. Pinning to loopback forces the + // kernel to detect the conflict and pick a different ephemeral port. + serverSocket = new ServerSocket(port, 50, java.net.InetAddress.getLoopbackAddress()); serverSocket.setSoTimeout(100); acceptThread = new Thread(() -> { @@ -269,10 +263,6 @@ public synchronized void sendClose(int code, String reason) throws IOException { writeFrame(WebSocketOpcode.CLOSE, payload, payload.length); } - public synchronized void sendPing(byte[] data) throws IOException { - writeFrame(WebSocketOpcode.PING, data, data.length); - } - private String computeAcceptKey(String key) { try { MessageDigest sha1 = MessageDigest.getInstance("SHA-1"); @@ -439,10 +429,6 @@ private boolean performHandshake() throws IOException { if (role != null) { sb.append("X-QuestDB-Role: ").append(role).append("\r\n"); } - int maxBatch = advertisedMaxBatchSize; - if (maxBatch > 0) { - sb.append("X-QWP-Max-Batch-Size: ").append(maxBatch).append("\r\n"); - } sb.append("\r\n"); out.write(sb.toString().getBytes(StandardCharsets.US_ASCII)); out.flush(); diff --git a/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java b/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java index 01ce0986..e9041448 100644 --- a/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java +++ b/core/src/test/java/io/questdb/client/test/impl/QueryWorkerTest.java @@ -24,11 +24,18 @@ package io.questdb.client.test.impl; +import io.questdb.client.Completion; import io.questdb.client.cutlass.qwp.client.QwpQueryClient; import io.questdb.client.impl.QueryWorker; import org.junit.Assert; import org.junit.Test; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + public class QueryWorkerTest { /** @@ -46,4 +53,112 @@ public void testClientGetterReturnsConstructorInstance() { Assert.assertSame(worker.client(), worker.client()); } } + + /** + * Regression test for the shutdown-vs-dispatch race in + * {@code QueryWorker.runLoop()}. If {@code shuttingDown} flips to true + * after {@code dispatch()} has set {@code current = q} but before the + * worker thread observes the wakeup, the run loop returns at the + * {@code if (shuttingDown) return;} branch without ever invoking + * {@code runOn(client)} or {@code signalUnexpected(...)}. The caller's + * {@link Completion#await()} would then block forever because + * {@code signalDone} is never called. + *

+ * Rather than try to win a timing race, this test reproduces the buggy + * state directly: it parks the worker on its condition, then takes the + * worker's own {@code signalLock} and atomically sets both + * {@code current} and {@code shuttingDown} before signalling. After the + * worker thread exits, the test asserts the {@link Completion} has been + * signalled. Today the assertion fails because the run loop's early + * return strands the {@code QueryImpl}. + */ + @Test(timeout = 30_000) + public void testShutdownRacingDispatchMustNotStrandCaller() throws Exception { + Class queryImplClass = Class.forName("io.questdb.client.impl.QueryImpl"); + Class poolClass = Class.forName("io.questdb.client.impl.QueryClientPool"); + + Field lockF = QueryWorker.class.getDeclaredField("signalLock"); + Field condF = QueryWorker.class.getDeclaredField("signalCondition"); + Field currentF = QueryWorker.class.getDeclaredField("current"); + Field shuttingF = QueryWorker.class.getDeclaredField("shuttingDown"); + Field threadF = QueryWorker.class.getDeclaredField("thread"); + for (Field f : new Field[]{lockF, condF, currentF, shuttingF, threadF}) { + f.setAccessible(true); + } + + Field doneF = queryImplClass.getDeclaredField("done"); + Field completionF = queryImplClass.getDeclaredField("completion"); + doneF.setAccessible(true); + completionF.setAccessible(true); + + // No QwpQueryClient is constructed here: runLoop exits at the + // shuttingDown check before reaching the first reference to + // {@code client} or {@code pool}, so passing null for both is fine + // and keeps the test cleanly isolated from any network or socket state. + QueryWorker worker = new QueryWorker(null, null, 0); + Thread t = (Thread) threadF.get(worker); + t.start(); + + ReentrantLock lock = (ReentrantLock) lockF.get(worker); + Condition cond = (Condition) condF.get(worker); + + // Wait until the worker thread is parked on its signalCondition. + long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (true) { + boolean parked; + lock.lock(); + try { + parked = lock.hasWaiters(cond); + } finally { + lock.unlock(); + } + if (parked) { + break; + } + if (System.nanoTime() > deadlineNanos) { + Assert.fail("worker thread never parked on its signalCondition"); + } + Thread.sleep(1); + } + + // Construct a QueryImpl with done=false, mimicking the state set up + // by QueryImpl.submit() just before it calls worker.dispatch(). + Constructor ctor = queryImplClass.getDeclaredConstructor(poolClass); + ctor.setAccessible(true); + Object queryImpl = ctor.newInstance(new Object[]{null}); + doneF.setBoolean(queryImpl, false); + Completion completion = (Completion) completionF.get(queryImpl); + + // Atomically force the racy state under the worker's own lock: + // current set AND shuttingDown set before the worker wakes. + lock.lock(); + try { + currentF.set(worker, queryImpl); + shuttingF.setBoolean(worker, true); + cond.signalAll(); + } finally { + lock.unlock(); + } + + // The worker thread must exit (it has observed shuttingDown). + t.join(5_000); + Assert.assertFalse("worker thread did not exit after shuttingDown=true", + t.isAlive()); + + // The Completion must have been signalled. Without the fix, await(2s) + // returns false because signalDone is never called. + boolean completed; + try { + completed = completion.await(2, TimeUnit.SECONDS); + } catch (RuntimeException expectedAfterFix) { + // Once fixed, the worker is expected to call signalUnexpected + // with a QueryException("QuestDB handle is closed") which + // await() rethrows. Either form of "completed" is acceptable; + // the bug is the silent hang. + completed = true; + } + Assert.assertTrue("BUG: QueryWorker.runLoop returned with shuttingDown=true " + + "while current!=null, never invoking runOn or signalUnexpected. " + + "The caller's Completion.await() hangs forever.", completed); + } } From 19225300f8dc9aba0ff5d5234e3f29d3d05defd1 Mon Sep 17 00:00:00 2001 From: bluestreak Date: Fri, 22 May 2026 22:25:00 +0100 Subject: [PATCH 11/12] review & bugfix --- .../http/client/WebSocketSendBuffer.java | 8 +- .../qwp/client/NativeBufferWriter.java | 12 +- .../cutlass/qwp/client/QwpBufferWriter.java | 4 +- .../cutlass/qwp/client/QwpEgressIoThread.java | 54 +++--- .../cutlass/qwp/client/QwpQueryClient.java | 8 +- .../client/SegmentedNativeBufferWriter.java | 4 +- .../io/questdb/client/impl/QueryImpl.java | 53 ++++-- .../io/questdb/client/impl/QuestDBImpl.java | 4 +- .../client/test/impl/QueryImplResetTest.java | 180 ++++++++++++++++++ 9 files changed, 270 insertions(+), 57 deletions(-) create mode 100644 core/src/test/java/io/questdb/client/test/impl/QueryImplResetTest.java diff --git a/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketSendBuffer.java b/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketSendBuffer.java index cc25beda..b9598c78 100644 --- a/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketSendBuffer.java +++ b/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketSendBuffer.java @@ -312,8 +312,8 @@ public void putShort(short value) { * Writes a length-prefixed UTF-8 string. */ @Override - public void putString(String value) { - if (value == null || value.isEmpty()) { + public void putString(CharSequence value) { + if (value == null || value.length() == 0) { putVarint(0); return; } @@ -326,8 +326,8 @@ public void putString(String value) { * Writes UTF-8 encoded bytes directly without length prefix. */ @Override - public void putUtf8(String value) { - if (value == null || value.isEmpty()) { + public void putUtf8(CharSequence value) { + if (value == null || value.length() == 0) { return; } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java index 274ce5eb..add22fb0 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java @@ -61,7 +61,7 @@ public NativeBufferWriter(int initialCapacity) { * @param s the string (may be null) * @return the number of bytes needed to encode the string as UTF-8 */ - public static int utf8Length(String s) { + public static int utf8Length(CharSequence s) { return s == null ? 0 : Utf8s.utf8Bytes(s); } @@ -225,8 +225,8 @@ public void putShort(short value) { * Writes a length-prefixed UTF-8 string. */ @Override - public void putString(String value) { - if (value == null || value.isEmpty()) { + public void putString(CharSequence value) { + if (value == null || value.length() == 0) { putVarint(0); return; } @@ -266,8 +266,8 @@ public void putString(String value) { * Writes UTF-8 bytes directly without length prefix. */ @Override - public void putUtf8(String value) { - if (value == null || value.isEmpty()) { + public void putUtf8(CharSequence value) { + if (value == null || value.length() == 0) { return; } @@ -338,7 +338,7 @@ private static void writeVarintDirect(long addr, long value) { Unsafe.getUnsafe().putByte(addr, (byte) value); } - private void encodeUtf8(String value, int utf8Len) { + private void encodeUtf8(CharSequence value, int utf8Len) { position += Utf8s.strCpyUtf8(value, bufferPtr + position, utf8Len); } } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpBufferWriter.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpBufferWriter.java index 5aa72ae6..d3baf938 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpBufferWriter.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpBufferWriter.java @@ -113,14 +113,14 @@ public interface QwpBufferWriter extends ArrayBufferAppender { * * @param value the string to write (may be null or empty) */ - void putString(String value); + void putString(CharSequence value); /** * Writes UTF-8 encoded bytes directly without length prefix. * * @param value the string to encode (may be null or empty) */ - void putUtf8(String value); + void putUtf8(CharSequence value); /** * Writes an unsigned variable-length integer (LEB128 encoding). diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpEgressIoThread.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpEgressIoThread.java index 5d961abd..9752ace8 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpEgressIoThread.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpEgressIoThread.java @@ -84,6 +84,14 @@ public class QwpEgressIoThread implements Runnable, WebSocketFrameHandler { // the I/O thread typically parks on this for the duration of the user's // onBatch callback, which is microseconds for a no-op consumer. private final QwpSpscQueue pendingRelease = new QwpSpscQueue<>(1); + // Reusable request holder. The user thread mutates fields and offers the + // same instance into {@link #requests} on every submit; the I/O thread + // reads the fields synchronously in {@link #sendQueryRequest} and does not + // retain a reference past that call. Reuse avoids a per-submit allocation + // -- one in-flight query per client makes this safe: the worker that + // mutates pendingRequest is blocked on the events queue until the I/O + // thread has finished consuming the previous instance. + private final QueryRequest pendingRequest = new QueryRequest(); // Single-slot request queue (Phase-1 allows one in-flight query). private final BlockingQueue requests = new ArrayBlockingQueue<>(1); private final NativeBufferWriter sendScratch = new NativeBufferWriter(); @@ -368,14 +376,20 @@ public void shutdown() { * payload contains; zero when the user supplied no binds. */ public void submitQuery( - String sql, + CharSequence sql, long requestId, long initialCredit, int bindCount, long bindPayloadPtr, long bindPayloadLen ) throws InterruptedException { - requests.put(new QueryRequest(sql, requestId, initialCredit, bindCount, bindPayloadPtr, bindPayloadLen)); + pendingRequest.sql = sql; + pendingRequest.requestId = requestId; + pendingRequest.initialCredit = initialCredit; + pendingRequest.bindCount = bindCount; + pendingRequest.bindPayloadPtr = bindPayloadPtr; + pendingRequest.bindPayloadLen = bindPayloadLen; + requests.put(pendingRequest); } /** @@ -687,14 +701,12 @@ private void sendCredit(long requestId, long additionalBytes) { * happens on this thread. */ private void sendQueryRequest(QueryRequest req) { - byte[] sqlBytes = req.sql.getBytes(StandardCharsets.UTF_8); sendScratch.reset(); sendScratch.putByte(QwpEgressMsgKind.QUERY_REQUEST); sendScratch.putLong(req.requestId); - sendScratch.putVarint(sqlBytes.length); - for (byte b : sqlBytes) { - sendScratch.putByte(b); - } + // putString writes varint(utf8 length) + utf8 bytes in one pass, + // straight into the native send buffer -- no intermediate byte[]. + sendScratch.putString(req.sql); sendScratch.putVarint(req.initialCredit); // 0 = unbounded (Phase-1 default) sendScratch.putVarint(req.bindCount); if (req.bindCount > 0 && req.bindPayloadLen > 0) { @@ -746,21 +758,19 @@ public interface TerminalFailureListener { void onTerminalFailure(byte status, String message); } + /** + * Mutable request holder reused across submits. Safe to reuse because at + * most one query is in flight per client: the worker thread mutates fields + * and offers the instance into {@link #requests}, then blocks on the events + * queue until the I/O thread has fully consumed the previous instance and + * delivered a terminal event. + */ private static final class QueryRequest { - final int bindCount; - final long bindPayloadLen; - final long bindPayloadPtr; - final long initialCredit; - final long requestId; - final String sql; - - QueryRequest(String sql, long requestId, long initialCredit, int bindCount, long bindPayloadPtr, long bindPayloadLen) { - this.sql = sql; - this.requestId = requestId; - this.initialCredit = initialCredit; - this.bindCount = bindCount; - this.bindPayloadPtr = bindPayloadPtr; - this.bindPayloadLen = bindPayloadLen; - } + int bindCount; + long bindPayloadLen; + long bindPayloadPtr; + long initialCredit; + long requestId; + CharSequence sql; } } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java index 5c5227b1..d21aeffd 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java @@ -963,7 +963,7 @@ public synchronized void connect() { * batches begin arriving on the new connection, and any rows delivered * before the reset should be discarded by the handler. */ - public void execute(String sql, QwpColumnBatchHandler handler) { + public void execute(CharSequence sql, QwpColumnBatchHandler handler) { execute(sql, null, handler); } @@ -978,7 +978,7 @@ public void execute(String sql, QwpColumnBatchHandler handler) { * supply the per-call values. Interpolating values into the SQL string * defeats this reuse. */ - public void execute(String sql, QwpBindSetter binds, QwpColumnBatchHandler handler) { + public void execute(CharSequence sql, QwpBindSetter binds, QwpColumnBatchHandler handler) { if (!executing.compareAndSet(false, true)) { throw new IllegalStateException( "QwpQueryClient.execute called while another execute is in flight; one query at a time per client"); @@ -1633,7 +1633,7 @@ private void connectToEndpoint(Endpoint ep) { } } - private void executeImpl(String sql, QwpBindSetter binds, QwpColumnBatchHandler handler) { + private void executeImpl(CharSequence sql, QwpBindSetter binds, QwpColumnBatchHandler handler) { if (closedFlag.get()) { throw new IllegalStateException("QwpQueryClient is closed"); } @@ -1750,7 +1750,7 @@ private void executeImpl(String sql, QwpBindSetter binds, QwpColumnBatchHandler * the user's handler in a {@link FailoverProbeHandler} so that the outer * loop can intercept transport failures before they reach the user. */ - private void executeOnce(String sql, QwpBindSetter binds, FailoverProbeHandler probe) { + private void executeOnce(CharSequence sql, QwpBindSetter binds, FailoverProbeHandler probe) { // Cache the I/O thread reference at entry: close() may null the field while // we are inside this loop, so reading the field per-iteration would NPE // exactly when the user is mid-execute() and close() races. The queue and diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/SegmentedNativeBufferWriter.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/SegmentedNativeBufferWriter.java index 54d81b79..458908f8 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/SegmentedNativeBufferWriter.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/SegmentedNativeBufferWriter.java @@ -139,12 +139,12 @@ public void putShort(short value) { } @Override - public void putString(String value) { + public void putString(CharSequence value) { currentChunk.putString(value); } @Override - public void putUtf8(String value) { + public void putUtf8(CharSequence value) { currentChunk.putUtf8(value); } diff --git a/core/src/main/java/io/questdb/client/impl/QueryImpl.java b/core/src/main/java/io/questdb/client/impl/QueryImpl.java index 6d493279..fc80d263 100644 --- a/core/src/main/java/io/questdb/client/impl/QueryImpl.java +++ b/core/src/main/java/io/questdb/client/impl/QueryImpl.java @@ -55,16 +55,16 @@ final class QueryImpl implements Query { private final Condition doneCondition; private final ReentrantLock doneLock = new ReentrantLock(); private final QueryClientPool pool; - private final QwpBindSetter wireBinds = this::applyBinds; - private final WrappingHandler wrappingHandler = new WrappingHandler(); private final StringSink sqlBuffer = new StringSink(); - private QwpBindSetter userBinds; - private QwpColumnBatchHandler userHandler; + private final WrappingHandler wrappingHandler = new WrappingHandler(); private volatile QueryWorker currentWorker; private volatile boolean done = true; private volatile String resultMessage; private volatile byte resultStatus; private volatile Throwable unexpectedError; + private QwpBindSetter userBinds; + private final QwpBindSetter wireBinds = this::applyBinds; + private QwpColumnBatchHandler userHandler; QueryImpl(QueryClientPool pool) { this.pool = pool; @@ -128,18 +128,6 @@ public Completion submit() { return completion; } - void runOn(QwpQueryClient client) { - client.execute(sqlBuffer.toString(), wireBinds, wrappingHandler); - } - - /** - * Signals an unexpected error from the worker thread (for example, an - * exception escaping {@code execute()} before any handler callback). - */ - void signalUnexpected(Throwable t) { - signalDone((byte) 0, t.getMessage() != null ? t.getMessage() : t.getClass().getSimpleName(), t); - } - private void applyBinds(QwpBindValues binds) { QwpBindSetter setter = userBinds; if (setter != null) { @@ -164,6 +152,39 @@ private void signalDone(byte status, String message, Throwable unexpected) { } } + /** + * Drops any prior builder state (SQL, binds, handler) if no submit is + * currently in flight. {@link QuestDBImpl#query()} invokes this before + * returning the per-thread instance so callers see the "reset to empty" + * contract documented on {@link io.questdb.client.Query} regardless of + * whether the previous use ended at a terminal handler callback or at + * {@link #abandon()}. + */ + void resetIfDone() { + if (done) { + userBinds = null; + userHandler = null; + sqlBuffer.clear(); + } + } + + void runOn(QwpQueryClient client) { + // Pass the StringSink directly as a CharSequence -- the wire encoder + // reads chars and writes UTF-8 bytes straight into the send buffer. + // sqlBuffer is stable for the duration of execute(): the calling + // worker thread is blocked here until a terminal event arrives, and + // sql(...) cannot be invoked again until done==true. + client.execute(sqlBuffer, wireBinds, wrappingHandler); + } + + /** + * Signals an unexpected error from the worker thread (for example, an + * exception escaping {@code execute()} before any handler callback). + */ + void signalUnexpected(Throwable t) { + signalDone((byte) 0, t.getMessage() != null ? t.getMessage() : t.getClass().getSimpleName(), t); + } + private final class InnerCompletion implements Completion { @Override diff --git a/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java b/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java index 672bd33b..cc974ac1 100644 --- a/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java +++ b/core/src/main/java/io/questdb/client/impl/QuestDBImpl.java @@ -114,7 +114,9 @@ public Query newQuery() { @Override public Query query() { - return queryThreadLocal.get(); + QueryImpl q = queryThreadLocal.get(); + q.resetIfDone(); + return q; } @Override diff --git a/core/src/test/java/io/questdb/client/test/impl/QueryImplResetTest.java b/core/src/test/java/io/questdb/client/test/impl/QueryImplResetTest.java new file mode 100644 index 00000000..1ff33b76 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/impl/QueryImplResetTest.java @@ -0,0 +1,180 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.impl; + +import io.questdb.client.Query; +import io.questdb.client.cutlass.qwp.client.QwpBindSetter; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatch; +import io.questdb.client.cutlass.qwp.client.QwpColumnBatchHandler; +import io.questdb.client.cutlass.qwp.client.QwpServerInfo; +import org.junit.Assert; +import org.junit.Test; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; + +public class QueryImplResetTest { + + /** + * Regression test for the state-carryover bug between consecutive + * submits on the per-thread {@code QuestDB#query()} handle. + *

+ * The Javadoc on both {@code Query} and {@code QuestDB#query()} promises + * that the returned instance is "reset to empty" / "in a reset state". + * Before the fix, {@code QuestDBImpl.query()} returned the bare + * thread-local without nulling {@code userHandler} / {@code userBinds}, + * so the second call below would silently reuse {@code h1}: + * 

+     *   db.query().sql("SELECT 1").handler(h1).submit().await();
+     *   db.query().sql("SELECT 2").submit();    // no .handler() -- reuses h1
+     *
+ * The {@code if (userHandler == null)} check in {@code submit()} could + * not catch the misuse because the field was still set from the prior + * submit. + *

+ * The fix is {@code QueryImpl.resetIfDone()}, invoked from + * {@code QuestDBImpl.query()} before the per-thread handle is returned. + * This test reaches into {@code QueryImpl} via reflection (the class is + * package-private and lives in a different package from this test) and + * asserts the reset clears all three configured fields when the prior + * run is in a terminal state. + */ + @Test + public void testResetIfDoneClearsBuilderStateInTerminalState() throws Exception { + Class queryImplClass = Class.forName("io.questdb.client.impl.QueryImpl"); + Class poolClass = Class.forName("io.questdb.client.impl.QueryClientPool"); + + Constructor ctor = queryImplClass.getDeclaredConstructor(poolClass); + ctor.setAccessible(true); + // QueryImpl never dereferences the pool outside of submit(); a null + // pool is fine for this state-only test. + Query q = (Query) ctor.newInstance(new Object[]{null}); + + // Mirror the post-submit().await() state: builder fields set, + // done flag true (the constructor default). + QwpColumnBatchHandler h = new NoopHandler(); + QwpBindSetter b = values -> { + // no-op + }; + q.sql("SELECT 1").binds(b).handler(h); + + Method reset = queryImplClass.getDeclaredMethod("resetIfDone"); + reset.setAccessible(true); + reset.invoke(q); + + Field handlerF = queryImplClass.getDeclaredField("userHandler"); + Field bindsF = queryImplClass.getDeclaredField("userBinds"); + Field sqlBufF = queryImplClass.getDeclaredField("sqlBuffer"); + handlerF.setAccessible(true); + bindsF.setAccessible(true); + sqlBufF.setAccessible(true); + + Assert.assertNull("userHandler must be cleared so a follow-up submit() without .handler() fails fast", + handlerF.get(q)); + Assert.assertNull("userBinds must be cleared so a follow-up submit() without .binds() does not reuse the prior setter", + bindsF.get(q)); + CharSequence sqlBuffer = (CharSequence) sqlBufF.get(q); + Assert.assertEquals("sqlBuffer must be empty so a follow-up submit() without .sql() throws 'sql is required'", + 0, sqlBuffer.length()); + } + + /** + * Symmetric guard: when a submit is in flight ({@code done == false}), + * {@code resetIfDone()} must NOT touch the configured fields. The + * dispatched worker thread is reading {@code sqlBuffer} in + * {@code runOn()} and {@code userHandler} via the wrapping handler; + * clearing them mid-flight would race. + */ + @Test + public void testResetIfDoneIsNoOpWhileSubmitInFlight() throws Exception { + Class queryImplClass = Class.forName("io.questdb.client.impl.QueryImpl"); + Class poolClass = Class.forName("io.questdb.client.impl.QueryClientPool"); + + Constructor ctor = queryImplClass.getDeclaredConstructor(poolClass); + ctor.setAccessible(true); + Query q = (Query) ctor.newInstance(new Object[]{null}); + + QwpColumnBatchHandler h = new NoopHandler(); + QwpBindSetter b = values -> { + // no-op + }; + q.sql("SELECT 1").binds(b).handler(h); + + // Flip the in-flight flag by setting done=false directly. + Field doneF = queryImplClass.getDeclaredField("done"); + doneF.setAccessible(true); + doneF.setBoolean(q, false); + + Method reset = queryImplClass.getDeclaredMethod("resetIfDone"); + reset.setAccessible(true); + reset.invoke(q); + + Field handlerF = queryImplClass.getDeclaredField("userHandler"); + Field bindsF = queryImplClass.getDeclaredField("userBinds"); + Field sqlBufF = queryImplClass.getDeclaredField("sqlBuffer"); + handlerF.setAccessible(true); + bindsF.setAccessible(true); + sqlBufF.setAccessible(true); + + Assert.assertSame("userHandler must survive resetIfDone() while a submit is in flight", + h, handlerF.get(q)); + Assert.assertSame("userBinds must survive resetIfDone() while a submit is in flight", + b, bindsF.get(q)); + CharSequence sqlBuffer = (CharSequence) sqlBufF.get(q); + Assert.assertEquals("sqlBuffer must survive resetIfDone() while a submit is in flight", + "SELECT 1", sqlBuffer.toString()); + } + + private static final class NoopHandler implements QwpColumnBatchHandler { + @Override + public void onBatch(QwpColumnBatch batch) { + } + + @Override + public void onEnd(long totalRows) { + } + + @Override + public void onEnd(long requestId, long totalRows) { + } + + @Override + public void onError(byte status, String message) { + } + + @Override + public void onError(long requestId, byte status, String message) { + } + + @Override + public void onExecDone(long requestId, short opType, long rowsAffected) { + } + + @Override + public void onFailoverReset(long requestId, QwpServerInfo newNode) { + } + } +} From 3f3c259c5f5395e6c7f557ba1d72aec50b723fca Mon Sep 17 00:00:00 2001 From: bluestreak Date: Fri, 22 May 2026 22:56:51 +0100 Subject: [PATCH 12/12] Add qwp to allowed PR title subtypes The parent questdb repo's PR title validator already accepts qwp as a subtype, but the client repo's validator did not. Add it here so QWP- scoped PRs can be titled consistently across the two repos. Co-Authored-By: Claude Opus 4.7 (1M context) --- ci/validate-pr-title/validate.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/validate-pr-title/validate.js b/ci/validate-pr-title/validate.js index 3ca8c9ca..f3e018fe 100644 --- a/ci/validate-pr-title/validate.js +++ b/ci/validate-pr-title/validate.js @@ -16,6 +16,7 @@ const allowedSubTypes = [ "log", "core", "ilp", + "qwp", "http", "conf", "utils",