feat(qwp): make the QuestDB facade a WebSocket (QWP) entry point and clean up the connect-string keys

[mtopolnik]

What

This branch makes the QuestDB facade QWP-over-WebSocket only and, ahead of
that, cleans up the ws/wss connect-string vocabulary so a single string is
parsed identically by the ingest Sender and the egress QwpQueryClient.

Facade rewrite

The facade (single-string connect, two-arg connect, and the builder's
ingestConfig/queryConfig) now requires the ws or wss schema and hands
the connect string verbatim to both clients. It previously routed single-string
ingest to HTTP via a schema-translation step, which was incorrect. http,
https, tcp, tcps, and udp are now rejected with a clear message.

A new parser package (io.questdb.client.impl) replaces ConfigStringTranslator:

• ConfigString tokenizes schema::k=v;k=v over the existing ConfStringParser.
• ConfigSchema is the single static ws/wss key registry, carrying each
  key's side, type, range, enum, and alias.
• ConfigView runs the reject pass (unknown key, plus a relocated-key hint for
  legacy http/tcp/udp keys), the typed getters, and an IPv6-aware addr
  parser with duplicate rejection.

QwpQueryClient.fromConfig reparses over ConfigView and now honors the
user/pass aliases; the username/password and token credentials
authenticate on both sides. Its validateConfig is reused by the facade's
fail-fast build path. Sender.fromConfig routes ws/wss to a new
fromConfigWebSocket driven by ConfigView. QuestDBBuilder validates both
strings up front (even when a pool min is 0 and nothing connects) and resolves
pool keys with explicit-wins precedence and cross-string conflict detection.

Connect-string vocabulary cleanup

All of this is QWP-only. QWP is unreleased, so none of it touches stable ILP.

Four keys are removed:

• auth -- the pre-formed HTTP Authorization header value. Credentials are
  now supplied only as the structured token or username/password keys, and
  the header is synthesized downstream (QwpQueryClient builds it the same way
  the Sender already did). The programmatic QwpQueryClient.withAuthorization(String)
  raw-header setter is removed too, so no path accepts a pre-formed header.
• path -- chose the egress upgrade URI (default /read/v1). The server
  accepts only /read/v1 (and its /api/v1/read alias), so the key selected
  between two synonyms and added no real configurability. The egress upgrade now
  uses the fixed /read/v1; the withEndpointPath setter is removed.
• in_flight_window -- a QWP store-and-forward concept that the parser only
  ever accepted as a no-op (it landed during the QWiP store-and-forward work, in
  the 1.1.0 beta line). The cursor / store-and-forward segment ring and append
  deadline govern backpressure, so the count had no effect. There was no
  programmatic setter.
• gorilla (gorilla=on|off, LineSenderBuilder.gorilla(), and the
  setGorillaEnabled/isGorillaEnabled plumbing) -- WebSocket ingestion now
  always uses Gorilla timestamp encoding (the encoder always sets FLAG_GORILLA);
  the column writer still falls back to raw values per column when delta-of-delta
  overflows int32. The UDP sender keeps its non-Gorilla path.

Key validation is tightened so the Sender and the QwpQueryClient treat every
key on a shared ws/wss string the same way:

• transaction (an ingress-only key) is now accepted and consumed by the egress
  parser, so a shared string carrying it no longer fails as unknown.
• the reserved error-policy keys on_internal_error, on_parse_error,
  on_schema_error, on_security_error, on_server_error, and on_write_error
  are accepted as no-ops on both sides, per the connect-string spec.
• the legacy ILP HTTP/TCP keys request_timeout, retry_timeout,
  request_min_throughput, and protocol_version, and the UDP-only keys
  max_datagram_size and multicast_ttl, are rejected on ws/wss (with a
  relocated-key hint) while remaining valid on their own transports.

Behavior changes / tradeoffs

• No breaking changes to released functionality. The QWP facade and the
  ws/wss connect-string surface are unreleased, so requiring ws/wss on
  the facade (and every vocabulary change above) is beta churn, not a break.
• Stable http/tcp/udp ILP is unaffected. Standalone Sender.fromConfig
  keeps every stable ILP key parsing exactly as before. The four removed keys are
  all QWP vocabulary; on those transports they were only ever tolerated as silent
  no-op passthroughs (auth, path, in_flight_window, so a shared QWP connect
  string would not error) or already rejected (gorilla, which was
  WebSocket-only and threw). They never affected ILP behavior there. After this
  change auth/path/in_flight_window are rejected as unknown keys on those
  transports, and the reserved on_*_error keys are additionally accepted as
  no-ops.
• Always-on Gorilla. WebSocket ingestion no longer offers gorilla=off.
  This path was WebSocket-only and unreleased.
• Duplicate keys. On the QWP paths, duplicate non-multi keys resolve
  last-write-wins, and the egress side now rejects duplicate addresses instead of
  silently accumulating them.
• Numeric values on the QWP paths are parsed with QuestDB's Numbers parser,
  so they accept underscore separators (e.g. 1_000) and a trailing L on
  longs, which the previous Integer/Long.parseInt rejected.
• Error messages change to a uniform colon dialect with did-you-mean hints.

Test plan

• New ConfigViewTest and QwpConfigKeysTest (every registry key recognized by
  both clients; the relocated-key hint table is exactly the legacy keys;
  token_x/token_y stay plain unknowns).
• New per-key "honored" suites -- WsSenderConfigHonoredTest,
  QwpQueryClientConfigHonoredTest, PoolConfigHonoredTest -- assert each config
  key's value is applied (not merely accepted), each with a drift guard over the
  registry.
• Updated the ws/egress/facade suites for the removed keys, the colon-dialect
  messages, relocated-key hints, and last-write-wins behavior;
  ConfigStringTranslatorTest removed.
• The full client test suite passes.

🤖 Generated with Claude Code

[mtopolnik]

Tandem parent PR: questdb/questdb#7314 — these must merge together (the parent's submodule pointer references this branch).

[mtopolnik]

Reviewed at level 3 (full mission-critical pass: change-surface map, parser-correctness, ingress parity, egress parity + resource safety, test/coverage/conventions, and a fresh-context adversarial sweep, with per-finding source verification). This is high-stakes territory (public API + connect-string parsing), so level 3 is appropriate.

Critical

None. No correctness defect, resource leak, or undefined behavior was found in the client module. The two independent deep passes (structured parity agents + a no-checklist adversarial agent) converged on the same conclusion: parsing, validation, pool resolution, native-memory cleanup, and the always-on-Gorilla flag are all sound.

Specifically verified correct (not just "looks fine"):

• Range/bound logic (ConfigView.outOfRange/rangeMessage): Long.MIN/MAX_VALUE sentinels and minOpen/maxOpen are exact on every boundary (auth_timeout_ms>0, compression_level [1,22], max_batch_rows [1,1048576], buffer_pool_size>=1, failover_max_attempts>=1). No off-by-one.
• Numbers.parseInt overflow: throws (never wraps) for int keys whose registry max is OPEN_MAX; buffer_pool_size=3000000000 is rejected, not wrapped to negative.
• getHostPorts IPv6 + dedup (port + "/" + host): no forgeable collision, no valid address wrongly rejected.
• Resource safety (QwpQueryClient.fromConfig, QwpQueryClient.java:414–465): native bindValues is freed on every throw path via the try/catch → client.close(); nothing throws between construction and the try; tlsRootsPassword.toCharArray() is NPE-safe because validateConfig enforces tls_roots both-or-neither; parsedEndpoints.get(0) can't IndexOutOfBounds because validateConfig guarantees a non-empty addr first.
• Always-on Gorilla (QwpWebSocketEncoder): flags initializes to FLAG_GORILLA once; beginMessage saves/restores flags, setDeferCommit touches only FLAG_DEFER_COMMIT, and no reset() clears flags; bits are distinct, so the flag is never lost. The connect(...) overloads line up positionally after the gorillaEnabled parameter removal.
• UNSET=-1 sentinel (QuestDBBuilder): every pool setter rejects values that could collide with -1, so explicit-wins / conflict-detection resolve correctly.

Moderate

M1 — The PR's headline public API QuestDB.connect(...) has no direct test. (in-diff; test gap)
QuestDB.connect(CharSequence) and connect(ingest, query) (QuestDB.java:74, 87) appear only in QuestDBExamples.java (a main(), no @Test). They are thin delegators to the well-tested builder (return builder().fromConfig(s).build()), so the untested surface is ~2 lines and is covered transitively (e.g. testHttpSingleConfigRejected drives builder().fromConfig). Still, the change's stated purpose is to make connect() the WebSocket entry point, and a one-line smoke test (single-string connect("ws::...") against the existing mock, plus schema rejection of http/tcp via connect()) is cheap insurance against a future refactor of the delegation. Not blocking given the transitive coverage.

Minor

m1 — validateWsConfigString fail-fast misses the build()-only sf_durability != MEMORY guard. (in-diff)
QuestDBBuilder.build() validates the ingest string via validateWsConfigString, which runs fromConfig + configureDefaults + validateParameters — but the sf_durability ∈ {flush, append} rejection lives in Sender.build() at Sender.java:1403, after validateParameters. So QuestDB.builder().fromConfig("wss::addr=...;sf_durability=flush;sender_pool_min=0;...").build() returns a live handle with no error (min=0 → no sender is constructed); the "not yet supported" error is deferred to the first borrowSender(). This contradicts the contract the PR newly advertises in build()'s javadoc ("a malformed config fails here even when both pools have min == 0"). Self-correcting on first use, no leak/corruption, and only affects an explicitly-unsupported feature — but worth either covering in validateWsConfigString (assert sfDurability == MEMORY) or softening the javadoc claim.

m2 — gorilla key removal has no rejection test. (in-diff)
The other three removed keys each get an explicit "unknown configuration key" test (auth: QwpQueryClientFromConfigTest:218; path: :745 + LineSenderBuilderWebSocketTest:799; in_flight_window: :678). gorilla has none, and the drift guard (QwpConfigKeysTest) only iterates registry keys, so a regression that silently re-accepts gorilla=off would go uncaught. Add assertRejected("ws::addr=h:9000;gorilla=off;", "unknown configuration key: gorilla") on both sides.

m3 — WsSenderConfigHonoredTest asserts auto_flush=off is "honored", but that config is unbuildable on WebSocket. (in-diff; test quality)
wsConfigSnapshotForTest() is parse-only (no validateParameters). On WS, auto_flush=off → disableAutoFlush() sets autoFlushIntervalMillis=Integer.MAX_VALUE, which validateParameters rejects ("disabling auto-flush is not supported for WebSocket protocol", Sender.java:3812) — a rejection two other tests assert. The "honored" snapshot proves only "value reached a builder field," not "produces a working Sender." Drop/adjust the auto_flush=off case and reword the class's "actually applied" javadoc.

m4 — Pool-precedence tests assert only assertNotNull. (in-diff; test quality)
testExplicitPoolKeyWinsOverConflictingStrings and testBuilderCallAfterFromConfigOverridesPoolKeysFromString (QuestDBBuilderTest) only check that build() didn't throw — they never assert the resolved value. They'd pass even if precedence silently picked the wrong source (as long as no conflict triggered). Strengthen with poolConfigSnapshotForTest().get("acquire_timeout_ms") equal to the explicit value.

m5 — Honored drift guards protect a hand-maintained COVERED set, not the assertion list. (in-diff; test quality)
testHonoredCasesCoverEvery*RegistryKey checks COVERED.contains(key), where COVERED is a literal decoupled from the assertHonored(...) calls. The guard does fire for a brand-new uncovered key (good), but a dev can silence it by adding the name to COVERED while forgetting the actual assertion. Driving both off one {key→expected} map would make the guard match its javadoc.

m6 — PR labels are incomplete. Only enhancement is set. Per CLAUDE.md, add scope labels — ILP (and Core/Compatibility if applicable) — to match the client/connect-string scope.

Cross-repo coordination (verified handled — not a defect in this PR)

Removing the client's QwpWebSocketEncoder.setGorillaEnabled/isGorillaEnabled and QwpWebSocketSender.setGorillaEnabled/isGorillaEnabled breaks compilation of 5 parent-repo tests that import those client classes (QwpSenderE2ETest:4650, QwpWebSocketSenderReceiverTest:2254, QwpCursorBoundsCheckTest:549, QwpUdpAllTypesTest:992, QwpTimestampDecoderTest:107/419). I confirmed the tandem PR #7314 ("test(qwp): adapt facade E2E tests to the WebSocket-only client", same branch) modifies exactly those 5 files and bumps the java-questdb-client submodule pointer. So this is correctly coordinated. The only residual requirement is the one the PR comment already states: #55 and #7314 must merge together, and #7314's submodule pointer must reference #55's final merged SHA.

Summary

Verdict: Approve — no blocking correctness, concurrency, or resource issues in the client module; the cross-repo break is verified handled by tandem PR #7314.

• Tradeoffs, fairly stated: the PR is net-positive — one parser shared by both clients, a registry-driven key vocabulary with strong drift guards, and genuinely improved fail-fast validation. The deliberate behavior changes (stricter failover_backoff ordering, blank-credential rejection, last-write-wins duplicates, duplicate-addr rejection on egress, removed auth/path/in_flight_window/gorilla) are all QWP-only and QWP is beta/unreleased, so the stable ILP surface is untouched. The main gaps are in test assertions (m3–m5 prove wiring but not buildability/values) and a small fail-fast contract overreach (m1) — none of which affect runtime correctness.

Recommended before merge (all quick): add m1 (or relax the javadoc), m2 (gorilla rejection test), and ideally M1 (a connect() smoke test); m3–m6 are polish.

[mtopolnik]

Reviewing at level 3 (full mission-critical pass: 10-agent matrix — Rust agent N/A, no .rs files — plus per-finding source verification).

Verdict: Approve with minor changes. This is a clean, well-tested refactor. The 3-layer parser (ConfigString/ConfigSchema/ConfigView) is sound; correctness, concurrency, resource-management, and cross-context callsite analysis all came back clean after source-level verification. No Critical or correctness-blocking issues. The findings below are one Moderate (test strength) and a handful of Minor/Nit items.

---

Critical

None.

---

Moderate

M1 — testQueryPoolBuildFailureUnwindsSenderPool cannot fail on the bug it targets (in-diff, test) — core/src/test/java/io/questdb/client/test/QuestDBBuilderTest.java

The test's stated purpose is to prove that when the query pool fails to build, the already-warmed sender pool is closed rather than leaked. But it only asserts build() threw — its own comment concedes this is an inference ("the build failing tells us the sender-pool unwind ran"). If build() threw and leaked the connected senders, the test still passes green. Since resource-unwind on the error path is the codebase's stated pain point, this should assert the unwind observably.

The underlying unwind logic is correct (QuestDBImpl ctor closes the sender pool in its catch; pre-existing, not changed by this PR — verified), so this is a test-strength gap, not a live leak. Fix: have the ingest TestWebSocketServer count live connections (or add a @TestOnly closed-count hook on the sender pool) and assert it returns to 0 after the failed build().

---

Minor

m1 — addr port rejects underscores while every other numeric key accepts them; ingress regression (in-diff) — core/src/main/java/io/questdb/client/impl/ConfigView.java:231

The PR advertises "numeric values now accept underscore separators (1_000) and a trailing L" via Numbers.parseInt/parseLong (used in getInt/getLong, ConfigView:158/176). But parsePort uses Integer.parseInt (:231), which rejects both. So within one string, auth_timeout_ms=2_000 parses but addr=h:9_000 throws invalid port in addr.

It is also a verified ingress regression: the old ws ingress addr parser (addAddressEntry, merge-base Sender.java:2763) used Numbers.parseInt, so ws::addr=localhost:9_000; parsed before and fails now. (Egress always used Integer.parseInt, so egress is unchanged.) Real-world impact is low — underscore-separated ports are exotic — but it's a genuine inconsistency with the PR's own feature. Fix: use Numbers.parseInt in parsePort for consistency (restores ingress behavior), or explicitly document that ports don't take separators.

m2 — Alias + canonical silently collide by position; duplicate keys validate only the last occurrence (in-diff) — core/src/main/java/io/questdb/client/impl/ConfigView.java:79,192

user→username and pass→password normalize into the same canonical slot, and getStr resolves last-write-wins by scanning backwards. So ws::addr=h:9000;username=alice;user=bob;password=p; authenticates as bob on both clients, silently discarding username=alice — no error, no warning. Reachable when config fragments are concatenated. Same root cause loosens validation: compression=BOGUS;compression=raw is now accepted (old code validated every occurrence in its switch). This is consistent with the documented last-write-wins model, so it's defensible by-design — but the alias-collision case is a silent footgun and is untested (the honored suites test alias and canonical only in isolation). Consider: rejecting an alias and its canonical in the same string, or documenting the precedence.

m3 — PR description's "Stable ILP is unaffected" contradicts a later bullet (metadata) — The "Behavior changes / tradeoffs" section leads with "Stable http/tcp/udp ILP is unaffected," then three bullets later concedes auth/path/in_flight_window "are rejected as unknown keys" on those same transports where they were previously tolerated no-ops. Both are technically defensible (the keys never functioned on ILP — verified: they entered via QWP/QWiP beta PRs #17/#26 and httpPath has no connect-key), but a user with auth=… in a tolerated string now gets a hard parse failure. Fix: scope the headline to valid keys and surface the no-op-key rejection as the one stable-surface change.

m4 — PR under-labeled (metadata) — Only enhancement. This changes the stable ILP connect-string parser (http/tcp/udp key handling), so it should carry ILP and arguably Compatibility. (Note: those labels are defined on the parent questdb repo / its tandem PR #7314, not this submodule — apply there.)

m5 — Test gaps (test) — All Minor:

• idle_timeout_ms=0 / max_lifetime_ms=0 via a connect string routes through the setter's 0→Long.MAX_VALUE mapping (correct but unverified — PoolConfigHonoredTest uses 4321/98765, never 0).
• housekeeper_interval_ms=50 via string should throw at the builder setter's >=100 bound — no test exercises a pool-key range rejection that originates in the setter (vs ConfigView).
• The underscore/L numeric-parsing feature has no test — one would have caught m1.
• The headline single-address connect(oneString) → both live pools path has no automated coverage in this submodule (the live test splits addresses; the PR defers it to the parent repo). Per "don't assume," confirm that parent-repo test exists and runs in CI.

---

Nits

• QwpQueryClient.java:844,854 — getAuthorizationHeaderForTest and configSnapshotForTest were inserted out of alphabetical order into a previously-sorted block (configSnapshotForTest ('c') is stranded among get* methods). Reorder to: configSnapshotForTest, getAuthorizationHeaderForTest, getAuthTimeoutMsForTest, getCompressionLevelForTest, …. (poolConfigSnapshotForTest / wsConfigSnapshotForTest parked at the end of their public blocks are a consistent @TestOnly pattern — leave them.)
• QwpQueryClient.fromConfig parses addr twice (once in validateConfig, once to build endpoints) — harmless cold-path redundancy.
• PR title (~99 chars) bundles two themes; optional trim to feat(qwp): make the QuestDB facade a WebSocket entry point.

---

---

Summary

• Verdict: Approve — address M1 (test) and m1 (port-underscore consistency); the rest are optional polish.
• Tradeoffs: the PR is honest about its behavior changes but the description's lead bullet (m3) oversells "stable ILP unaffected"; the always-on Gorilla and underscore-parsing changes are net-positive but the latter is applied inconsistently (m1). No performance regression on any data path — the new parser is strictly connect/build-time and discarded immediately.

[mtopolnik]

[PR Coverage check]

😍 pass : 675 / 691 (97.68%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/Sender.java	195	204	95.59%
🔵	io/questdb/client/impl/ConfigString.java	27	28	96.43%
🔵	io/questdb/client/QuestDBBuilder.java	74	76	97.37%
🔵	io/questdb/client/cutlass/qwp/client/QwpQueryClient.java	139	142	97.89%
🔵	io/questdb/client/impl/ConfigView.java	134	135	99.26%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketEncoder.java	3	3	100.00%
🔵	io/questdb/client/impl/Side.java	6	6	100.00%
🔵	io/questdb/client/impl/ConfigSchema.java	97	97	100.00%

[bluestreak01]

Approved after a level-3 mission-critical pass (full source verification + local test run + CI confirmation on HEAD).

No critical, concurrency, or resource issues. Verified:

• Always-on Gorilla flag never lost (init FLAG_GORILLA, beginMessage saves/restores, no reset clears it).
• UNSET=-1 pool sentinel is unproducible by any setter; timeouts map only 0->MAX.
• fromConfig native (bindValues) cleanup on every throw path; validateConfig pre-validates before construction.
• Single-string facade port parity (ingress/egress both 9000) and blank-credential parity (matching messages on both sides).
• JDK 8 floor: no Java 9+ APIs in changed files; the one Map.of slip was caught by CI and fixed (98269c8). CI green on HEAD across both JDK fronts.
• 388+ targeted tests green locally.

All prior-review findings (port underscore, gorilla rejection test, connect() smoke test, observable sender-pool unwind, pool-precedence value assertion, sf_durability fail-fast) are resolved in later commits.

Minor/optional nits only: @testonly getter ordering in QwpQueryClient (getAuthorizationHeaderForTest/configSnapshotForTest out of alphabetical order) and a defensive null-guard in ConfigView.getEnum.

Reminder: must merge together with the tandem parent PR questdb/questdb#7314, whose submodule pointer must reference this PR's final merged SHA.