Repository navigation
feat(qwp): add refreshing token providers for Entra ID authentication - #102
Open
bluestreak01 wants to merge 27 commits into
Open
bluestreak01 wants to merge 27 commits into
bluestreak01 wants to merge 27 commits into
Conversation
Add a cross-language specification for QWP clients that authenticate with rotating bearer tokens (Microsoft Entra ID managed identities and service principals), plus the Java findings and implementation plan. - design/qwp-token-provider-spec.md (v0.3, decisions resolved): the token-source contract, a proactively refreshing shared token cache, when clients fetch tokens, failure policy by phase (one retry after 401), connect-string keys (token_provider, azure_resource, azure_client_id), connection health, an optional authentication- outage deadline, redaction rules and conformance tests C1-C23. - design/entra-id-qwp-auth.md: findings for the Java client with code references, the design rationale, and the four-step Java plan (§10). Security-relevant server findings are reported privately and are not part of this change.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Implement the dynamic-credential specification (design/qwp-token-provider-spec.md, v0.3) and the four-step Java plan in design/entra-id-qwp-auth.md, section 10. - Token cache: RefreshingTokenProvider, ExpiringToken, TokenSource and TokenUnavailableException. Proactive jittered refresh at about half the token lifetime, a 60 s hand-out floor, single-flight cold waits bounded by cold_wait, jittered backoff honouring Retry-After, rate-limited forced refresh, and token redaction everywhere. - Client integration: one immediate same-endpoint retry after a refreshable 401 (WWW-Authenticate aware; never for a 403 or a static credential) on every ingest connect path, orphan drains included, and on egress connect and failover. SYNC startup retries a retryable provider failure within its budget (D6); any other provider exception still fails fast (D8). Egress errors name the failure class, and a query client whose failover reconnect failed reconnects on its next execute() instead of staying unusable (the suspected dead pooled worker, reproduced by a test before the fix). - Connect string: token_provider, azure_resource and azure_client_id on both clients (wss:: only; exclusive with static credentials and application-supplied providers), resolved through a ServiceLoader SPI and a process-wide ref-counted registry with a 60 s linger. Validation never fetches a token. - New optional module azure/ (org.questdb:questdb-client-azure): token_provider=azure on DefaultAzureCredential with the spec's error classification. Java 8 floor, released together with the client. - Connection health: Sender.health(), QwpQueryClient.health() and an aggregate QuestDB.health(); an optional auth_failure_max_duration_millis deadline for authentication outages. - Tests for conformance scenarios C1-C23, plus a TLS mode for the test WebSocket server (its key is generated at test time). README and design docs updated; the spec's Appendix B now names the published artifact org.questdb:questdb-client-azure.
bluestreak01
force-pushed
the
feat/qwp-entra-token-provider
branch
from
October 2, 2026 10:15
576383b to
c9f2b96
Compare
The spec classified Azure Identity's "no credential available in the chain" as permanent (section 7.5), while section 4 calls network failures and IMDS 404/410 retryable. Inside DefaultAzureCredential an unreachable IMDS produces exactly that message: the chain probes IMDS once, with a short timeout and no retries, so a transient IMDS outage failed a SYNC startup fast. Spec v0.4 follows Azure Identity's own split between fail-fast discovery and a resilient single credential: - 7.1, 7.2: a new key, azure_credential (default, managed_identity, workload_identity, environment), with its validation rules. - 7.5: library errors are classified by how the credential was selected. With one credential, configuration errors are permanent and endpoint or network failures retryable. In the discovery chain, "no credential available" is retryable and the client warns once. Errors should carry the library's innermost reason. - 4: a source that discovers its credential by probing must not treat "nothing found" as permanent on that evidence alone. - 10: conformance test C24. 12: decision D10, which records why an SMBIOS host check was rejected. Appendix B: the Java binding. Appendix D: precedents from Azure Identity, MongoDB, Google, AWS and Apache Druid. The Java implementation still follows v0.3.
Implements spec v0.4's azure_credential and aligns the spec with the code, so the two no longer disagree. - azure_credential=default|managed_identity|workload_identity|environment is accepted by the Sender, QwpQueryClient and the QuestDB facade, and validated per spec 7.2: an unknown value lists the supported ones, and azure_client_id cannot be combined with environment. default is normalized away, so existing connect strings take the same code path and share providers as before. - The other values build that credential directly. Passing AZURE_TOKEN_CREDENTIALS to DefaultAzureCredentialBuilder.configuration(), as Appendix B prescribed, does not work in azure-identity 1.18: it selects the credential but keeps the chain's one-shot IMDS probe, because each credential re-reads the selector from the process-wide configuration. A managed identity built directly retries an unreachable endpoint and rides out an outage. - A managed identity used alone reports "identity not assigned" only in a nested MSAL message. AzureTokenSource now classifies it as permanent; otherwise a sync startup retries that misconfiguration for its whole connect budget (measured: 316 s instead of ~25 s). - Retryable errors lead with their socket-level cause, such as a refused connection, which managed identity otherwise hides behind "see inner exception". Only java.net socket messages are echoed. - The discovery chain's "no credential available" stays permanent, so a host without a credential still fails fast. Instead, the provider warns once when DefaultAzureCredential starts as a chain, recommending azure_credential in production. - A workload identity missing its platform configuration no longer fails inside a client's build(): every fetch fails as permanent. The spec is amended to match (sections 4, 7.5, C24, D10, Appendix B and D), and entra-id-qwp-auth.md now points at v0.4. Tests: AzureManagedIdentityLibraryTest runs the real library in a child JVM against a local IMDS stub (MSAL reads the endpoint only from the process environment) for C24: an outage ridden out, an unreachable endpoint retryable, an unassigned identity permanent, the chain failing fast and warning once, and the platform selector silencing the warning. Plus AzureCredentialSelectionTest, new AzureTokenSourceTest cases, and TokenProviderConfigTest and config-honored coverage for the new key.
Brings in #104 (pool-wide sf_max_total_bytes budget and graceful sender close). Conflict in Sender.java: this branch moved the WebSocket build() branch into buildWebSocket(HttpTokenProvider), while #104 edited that block in place. Resolved by keeping the move and applying #104's edits to buildWebSocket: resolveSfMaxSegmentBytes()/resolveSfMaxTotalBytes(), newCursorEngine(..., sfSharedBudget, ...) for both engine constructions, and the sfSharedBudget argument on both quarantineTornSlot() calls. A token_provider sender built by the pool now charges the pool's shared segment budget like any other pooled sender.
QueryWorker.shutdown() closed the query client once its bounded 5 s join expired, even with the dispatch thread still inside execute(). A reconnect walk parked in a native connect or WebSocket upgrade does not see the shutdown interrupt, so it outlived the close: it went on to a healthy endpoint, then wrote the binds into the bind buffer close() had freed (SIGSEGV), or published an I/O thread and connection on the closed client (leak). The failover reconnect could already reach this; the reconnect on the next execute() after a failed failover made it reachable from every query on such a client. When the join expires with the dispatch thread still busy, the thread now closes its own client on its way out of runLoop(). One CAS decides which side closes, so the client is closed exactly once and never under a running execute(). Shutdown latency is unchanged and the per-query path is untouched.
executeImpl's failover-budget-exhausted exit returned without telling the connection health tracker, unlike its sibling exits. A query whose connection died on the last attempt the budget allowed, or after failover_max_duration_ms ran out, left QwpQueryClient.health() - and the QuestDB.health() aggregate - reporting CONNECTED until the next execute(). With failover_max_attempts=1 every later query fails on the same latched failure without reconnecting, so the client reported CONNECTED for its whole life while each query failed. That exit now records the loss with connectionLost(), as the other exits do: RECONNECTING, since the next execute() fails over. With a single attempt per execute() it also calls failed(): the client never reconnects, as with failover=off, so FAILED is the true state. Health is read only through health(), so query behaviour is unchanged, and the success path never reaches this code. Tests: ConnectionHealthTest covers a connection lost on the last allowed attempt (RECONNECTING, then CONNECTED on the next query) and with a single attempt (FAILED, and the next query fails without reconnecting).
The auth_failure_max_duration_millis javadoc and the ERROR log the deadline emits promised that unacknowledged rows stay in on-disk store-and-forward for a later sender or an orphan drain. That only holds with sf_dir: the option is accepted without one, memory-mode segments are malloc'd, orphan drains require sf_dir, and spec section 8.5 says memory-only data is lost when the sender closes. Document both modes in the javadoc, and make the log line name the outcome for the sender's actual backing (engine.sfDir() is null only in memory mode).
Contributor
|
Tandem: questdb/questdb#7799 (submodule bump to |
Bring the symbol-dictionary recycle API into the Entra token\nprovider branch so the OSS test suite compiles against the pinned\nclient. Preserve both the token-provider handshake and recycle setup\nin the sender builder, and carry lifetime counters into the loop.\n\nThe client and OSS test compiles pass with JDK 25; targeted client\nand OSS integration tests exercise both features together.
authOutageDeadlineFired() built its TERMINAL SenderError span from the engine's raw FSNs, while every other SenderError the loop emits adds externalFsnBase. After a symbol-dictionary recycle, which is on by default, the rebuilt engine numbers its frames from 0 again, so the span named FSNs that flushAndGetSequence() had already returned for acknowledged rows and left out the rows still waiting. The span now adds externalFsnBase like its siblings. ConnectionHealthTest gains a test that recycles the dictionary, then rejects every handshake with 401 past a short deadline, and checks the terminal span against the FSNs the sender reported. It fails without the fix (fromFsn 0 instead of 1), and also fails if the rebuilt loop is not armed with the deadline. QwpQueryClientDynamicCredentialTest gains three tests for paths that no test pinned: - connect() and a failover reconnect retry a refreshable 401 only once, even when the provider hands out a new token on every pull. Without the limit, both walks kept retrying for as long as the server rejected. - When the reconnect that the next execute() makes after a failed failover fails as well, the operation reports "reconnect failed:", and a later execute() still reconnects instead of throwing "not connected".
glasstiger
changed the title
The SYNC budget-exhaustion test in WebSocketDynamicCredentialTest asserted more than two token pulls within a 300 ms connect budget, with 10-40 ms backoff parks between pulls. On the mac-aarch64 CI runner a timed park wakes about 100 ms late: the sibling test with the same backoff spent 196-504 ms in three parks across five mac runs, against 67-72 ms on linux and windows. The budget then fit only two pulls, and the assertion failed. Running the test JVM under background QoS (taskpolicy -b) reproduces the overshoot locally: a 10 ms park takes ~105 ms, the 300 ms budget yields exactly three pulls in every run, and added CPU load drops it to one or two. A 2 s budget yields 15-17 pulls under the same load. The test now gives the connect loop 2 s. Its assertions are unchanged and the client code is untouched: the loop honours the budget and retries; only the test assumed timely wake-ups.
TestWebSocketServer counted a validator rejection only after it had flushed the 401, so a test that reads authRejectCount() as soon as the client has handled the rejection could see the count one short. That is how testPersistent401AfterTheRetryFailsStartup failed on the JDK 8 CI job (expected 2, was 1). The server now counts before it writes the response. With a 50 ms lag added after every reject, all 40 tests in the three classes that read the counter pass; without the reorder the same lag fails that test every time. testDeadlineIsNotResetOrFiredByOtherFailureClasses said that role rejects and transport failures do not reset the auth-outage clock, but it only waited for some terminal, so a loop that reset the clock in those branches still passed it. The test now parses the elapsed time from the terminal and requires it to span the role-reject and transport rounds. It fails against such a reset and passes without it. Comments and javadoc still said that an eager initial connect fails fast with the provider's own exception and that a running sender never latches a terminal on a credential outage. They now describe the SYNC retry of a retryable provider failure, the LineSenderException that reports an exhausted budget, the extra token pull after a 401 and the opt-in auth_failure_max_duration_millis deadline. The merge of main had also brought back, at the build() callsite, the recovery comment that #91 moved into constructEngineOnSlotLocked, and had dropped two #91 comments; main's text is restored there.
Contributor
Review at
|
glasstiger
marked this pull request as ready for review
October 14, 2026 13:42
ConnectionHealth and QwpConnectionHealthTracker declared lastFailure before lastConnectedAt(EpochMillis), and ConnectionHealth listed getLastFailure() before getLastConnectedAtEpochMillis(). This commit puts the fields and getters in alphabetical order, as the member sorting convention requires. WsSenderConfigHonoredTest now writes the expected auth_failure_max_duration_millis value as 600_000L, using the thousands separator for numbers of five or more digits. No behaviour changes. ConnectionHealthTest and WsSenderConfigHonoredTest pass.
Four fixes from the level-3 review of this branch. README: the auth_failure_max_duration_millis note promised that unacknowledged rows stay on disk when the deadline fires. That holds only with sf_dir. A memory-only sender, the default, loses them when it closes, as the javadoc, the deadline's log line and spec section 8.5 already say. The README now names both outcomes. RefreshingTokenProvider: a fetch whose token expired within handout_floor (60 s) of its receipt counted as a success, although the provider can never hand such a token out. A source that issues tokens that short, such as a Keycloak realm at its 1-minute minimum access-token lifespan, left every caller waiting out cold_wait for a "no usable token" error with no cause, no WARN, and a provider that reported no failures. checkResult() now treats such a result as a retryable failed fetch whose message names handout_floor, so the WARN log, getLastFailure() and the caller's error all carry it. Spec v0.5 amends section 4 and conformance rows C2 and C6 to match, and the README documents the floor and handoutFloorMillis(). azure/pom.xml: surefire ignored -Dtest.include and -Dtest.exclude, so a questdb server build with the local-client profile ran all 26 Azure tests in every CI shard: about 35 s in each of the 15 Linux shards of OSS build 279062. The module now maps both properties the way core does. Its defaults still run every test. RefreshingTokenProviderTest: the C8 step for a stale token ran within forced_min_interval, so the rate limit returned first and the test passed with the stale-token check deleted. The step now runs once the interval has passed and asserts that no fetch is scheduled. Verification: deleting the stale-token check now fails C8. Reverting the handout_floor check fails the new hand-out-floor test and the two updated C2 and C6 tests. With -Dtest.include='**/griffin/**' the azure module runs no tests (26 before), and its default selection still runs all 26.
RefreshingTokenProviderTest failed on the JDK 8 CI job with a
NullPointerException in the condition of the await() at line 298:
await(() -> scheduler.peek() != null
&& scheduler.peek().delayNanos == 0, ...)
The condition reads ManualScheduler.peek() twice while another thread
makes the provider replace its scheduled task. scheduleFetchLocked()
cancels the pending refresh before it schedules the forced fetch, and
peek() returns live tasks only, so between those two steps the
scheduler holds none: the first peek() saw the old task and the
second returned null.
ManualScheduler.isNextImmediate() now reads the task list once. The
four awaits that poll while a rejecter or cold-caller thread runs,
two in the C8 forced-refresh test and one in each hand-out-floor
test, use it. Every other peek() dereference runs after the
background call has been joined, or on the test thread alone.
With a 100 ms pause added between cancel() and schedule() in the
provider and a 20 ms pause in peek(), the old condition failed 3 of 3
runs with the CI stack trace, and the new one passed 3 of 3. Without
the pauses, RefreshingTokenProviderTest passes 5 of 5 runs.
The race dates from c9f2b96; 2c4a79b did not change these lines.
LineSenderBuilder.build() takes a TokenProviderRegistry lease before it connects a token_provider sender, and its finally block closes the lease when buildWebSocket() throws. No test covered that path: with the lease.close() call deleted, the core and azure suites still passed, while the failed build left the lease held, so the shared provider's refresher thread would keep fetching tokens for the life of the process. TokenProviderSharingTest gains testSenderBuildFailureReleasesTheLease. It builds a token_provider sender against a port nothing listens on, expects the connect failure, and checks that build() created the provider, that the lease count is back to zero, and that the provider lingers for the next attempt. With the lease.close() call deleted the test fails with "expected:<0> but was:<1>". WsSenderConfigHonoredTest now lists its TestTokenProviderFactory import after io.questdb.client.impl.Side, in alphabetical order.
The opt-in auth_failure_max_duration_millis let a running store-and-forward sender become terminal once credential-unavailable or 401/403 rounds had lasted that long. Its clock started at the first such failure and kept running through role-reject and transport rounds, so a failover between two brief credential failures could end a producer whose authentication never failed for long. A store-and-forward sender must never become terminal on a transient outage, so the deadline is removed. An established sender again retries every authentication-class failure indefinitely, and reports each one to the error handler and in its connection health. An application that prefers a hard failure closes the sender itself. The connect-string key, the builder method and the send loop's outage clock are gone, and the spec moves to v0.6: section 8.5, D7 and C22 now rule such a deadline out. ConnectionHealthTest replaces the deadline tests with testAuthenticationOutageNeverMakesTheSenderTerminal. It drives credential-unavailable, 401 and role-reject rounds, checks that the sender stays alive, and checks that it delivers the buffered row after recovery. With a terminal latched on a credential failure in the send loop, it fails with "expected:<RECONNECTING> but was:<FAILED>".
A sender whose I/O loop latched a terminal error while it was connected, such as a batch the server rejects with PARSE_ERROR, reported FAILED with outageSince NONE. CursorWebSocketSendLoop.recordFatal() reports only failed(), and failed() never set the outage start that upgraded() had cleared. ConnectionHealth documents NONE as "while connected", and the aggregate's oldest outage as NONE only when every connection is connected, so QuestDB.health() left such senders out of its oldest outage. A query client already calls connectionLost() before failed(). QwpConnectionHealthTracker.failed() now starts the outage when none is running. A client that was already without a connection keeps the start of the outage it is in. ConnectionHealthTest gains two tests for paths that no test pinned: - testSenderHealthIsFailedWithAnOutageAfterATerminalRejection: a server that rejects every batch with PARSE_ERROR moves the sender to FAILED with an outage start. It fails without the fix (outageSince -1), and with "expected:<FAILED> but was:<CONNECTED>" if recordFatal() stops reporting failed(). - testQueryClientHealthIsFailedWhenFailoverIsOffAndItLosesItsConnection: with failover=off a dropped connection leaves the query client FAILED, and the next query fails without reconnecting. It fails with "expected:<FAILED> but was:<RECONNECTING>" if the failover-off branch of executeImpl() stops reporting failed(). The new query test and the failover_max_attempts=1 test now share one helper, assertQueryClientFailsForGood().
glasstiger
added a commit
to questdb/documentation
that referenced
this pull request
Oct 14, 2026
… renewal and Java token providers Align the OIDC, RBAC and client pages with questdb/questdb-enterprise#1267, questdb/questdb#7799, questdb/ui#603 and questdb/java-questdb-client#102. - Issuer validation (QuestDB Enterprise 4.0.2): acl.oidc.issuer and acl.oidc.issuer.validation.enabled, a new "The expected issuer" section with where the issuer comes from, the issuers of Entra ID v1.0 and v2.0 tokens, the common/organizations templates, issuers that depend on the host name of the request, the opt-out and its advisory, the InvalidIssuer and MissingRequiredClaim("iss") log lines, and the upgrade notes. acl.oidc.host needs acl.oidc.issuer unless validation is disabled. PGWire tokens need the expected issuer too. - HTTP sessions: a request with a credential that QuestDB rejects, or cannot check because the provider is unreachable, gets 401 and ends its session. Sessions end with the password or REST token that opened them, or when the user is disabled or dropped, and on a replica when they are next used. acl.oidc.cache.ttl reuses only the check of a password that QuestDB verifies itself, even with OIDC disabled. - Provider outages: with the User Info endpoint, Web Console users are logged out once acl.oidc.cache.ttl has passed; with tokens validated locally, once the ID token expires; ROPC logins need the token endpoint in both modes. - Web Console: it renews the ID token by its exp claim, keeps its token while it cannot reach the provider, and without a refresh token logs the user out when the token of the login expires. Remove the stale "logged out about an hour after the login" text from the OIDC page and the Entra ID guide; Entra ID limits the refresh tokens of single-page applications to 24 hours. Its password login sends a REST token that survives a password change, so drop the user's REST tokens to log the user out. - Java client: "Rotating tokens" with token_provider=azure, the questdb-client-azure artifact, RefreshingTokenProvider and how token failures are handled, "Connection health", the token provider keys in the connect string reference, and a Java example in the Entra ID guide. - With the User Info endpoint, QuestDB treats the access token as opaque. - Changelog entries for the above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes for the three Moderate findings of the level-3 review of this branch at 4355127. RefreshingTokenProvider scheduled its proactive refresh from the token's lifetime alone and never consulted handout_floor. A token that lived less than about two minutes left its hand-out window before the refresh ran: a 100 s token was handed out for 40 s but refreshed 45-50 s after its fetch, so every caller in between waited for a fetch. The idle-client promise in the javadoc, the README and spec section 5.2 did not hold for such tokens. The schedule now starts the refresh at least p (min_refresh_interval, or half the lifetime when shorter) before the token leaves its hand-out window, but never sooner than p after the fetch. So min_refresh_interval stays a lower bound, and no source can drive a fetch storm. A token that lives 3 minutes or more keeps its schedule, so the existing C1 cases are unchanged. A token handed out for less than p still cannot be refreshed in time; the provider now logs that once, naming handout_floor. Spec v0.7 amends section 5.2 and C1, and the README and javadoc state the precondition. CursorWebSocketSendLoop.connectWithRetry stops retrying a retryable credential failure once the calling thread is interrupted, but no test pinned that. With the clause removed, all 421 tests that reach the SYNC connect path passed, while an interrupted SYNC build spun through its whole budget because every backoff park returned at once: 818,107 provider pulls in 3 s. WebSocketDynamicCredentialTest gains testSyncInitializationStopsRetryingWhenTheThreadIsInterrupted. Two health computations had no failing test either: - ConnectionHealth.Aggregate's oldest-outage and latest-failure selection could be inverted unnoticed, because the only facade test used connections that were all connected. - Dropping the send loop's connectionLost() call also passed every test, because each RECONNECTING assertion ran after a failed round, which sets that state too. Without the call, a sender reports CONNECTED for its whole first reconnect round. ConnectionHealthTest gains testAggregateReportsTheOldestOutageAndTheLatestFailure and testSenderHealthIsReconnectingFromTheMomentItLosesItsConnection. Verification: each new test fails with its fix reverted. The schedule tests fail with the hand-out window bound removed, the once-only warning test with the flag dropped (two warnings), the SYNC test with the interrupt clause removed, and the health tests with the comparisons inverted or connectionLost() removed. The 15 related core test classes (120 tests) and the azure module (26 tests) pass.
Tag scheduled linger callbacks with an entry generation so an already started callback cannot close a provider after a newer release resets the linger period. Add deterministic executor-gated tests for idle and active leases and verify the current callback still closes the provider. Capture worker failures in assertReturnsPromptly and report them on the test thread, preserving the cause. Cover runtime exceptions and errors. Validation: 128 tests pass on JDK 25 and 42 focused tests pass on JDK 8. The broader JDK 8 run passes all 128 with tiered compilation disabled. The default JDK 8 run crashes in Unsafe.recordMemAlloc; the same crash also reproduces on unchanged parent 8eaaef1.
Recheck the warm reader's token snapshot under the provider lock before pulling a scheduled refresh forward. Preserve the future schedule when another fetch has already published a new token. Add a deterministic concurrent-reader regression test and cover a successful refresh that outlasts a short token's handout window. Clarify the Javadoc, README and cross-language specification so continuous availability depends on refresh completion time, not source health alone. Validation: all 130 focused core and Azure tests pass on JDK 25 and JDK 8 with -Xint. The new race test fails without the fix.
Clamp the jittered delay to at least one millisecond so accepted one-millisecond backoff settings cannot schedule immediate retries. Preserve longer Retry-After hints and the immediate initial prefetch. Cover repeated failures across jitter samples and Retry-After values with the existing deterministic scheduler, and document the floor. Validation: 36 client tests pass on Java 8 with -Xint and Java 25.
Cover sender shutdown while a rejected token blocks the rejection callback or the second credential pull. Assert that close interrupts each wait and completes normally. Cover provider shutdown with two cold callers and an unfinished fetch. Keep fetch completion separate from close notification so the test detects a missing signal or waking only one waiter. Run 89 related tests on JDK 25 and Java 8 with -Xint. Confirm the new tests fail against cancellation and notification mutations in an isolated worktree.
Preserve Retry-After headers from MSAL service exceptions, including\nwrapped failures. Service-principal credentials surface throttling through\nMSAL rather than HttpResponseException, so the adapter previously lost\nthe final response's delay after the SDK exhausted its retries. Share delta-seconds parsing between Azure HTTP and MSAL response headers\nwithout changing permanent-error classification or retaining SDK causes. Add regression tests through the real ClientSecretCredential and its\ndefault retry policy for throttling, server errors, wrapped exceptions,\ncase-insensitive headers, and missing or malformed hints. Verify MSAL\nfailures without response headers still use the provider's backoff. Validation: all 30 Azure tests and 35 refreshing-provider tests pass on\nnative arm64 JDK 8 and JDK 25. The new throttling tests fail before the\nproduction fix and pass with it.
Replay a canceled scheduled callback before its forced replacement using the existing fake clock and manual scheduler. Assert that the callback neither fetches a token nor replaces the pending task, and that the forced refresh publishes the new token exactly once. All 36 provider tests pass on JDK 25 and on JDK 8 with -Xint. Removing only the scheduled-generation guard makes the new test fail on an unexpected extra fetch while the other 35 tests still pass.
Restore RECONNECTING health when a replacement connection loses its\ntransport during dictionary catch-up. Publish the loss before backoff\nor another credential pull so health polling cannot report CONNECTED\nwhile the sender waits to reconnect. Keep the retry policy unchanged.\n\nAdd a real-socket regression test that populates the symbol dictionary\nthrough the sender API, fails the reconnect catch-up send, and holds\nthe following credential pull. Check the outage state, buffered delivery,\nand recovery. The test fails before the production fix.\n\nValidation: 178 targeted Maven tests pass on native ARM64 JDK 25,\nincluding health, cursor send-loop, credential cancellation, dynamic\ncredential, and JVM-error cleanup tests. git diff --check also passes.
Contributor
[PR Coverage check]😍 pass : 1232 / 1359 (90.65%) file detail
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tandem: questdb/questdb#7799, questdb/questdb-enterprise#1267
Merge order
expclaim.java-questdb-clientat this PR's squash commit onmain.What this is
Support for QWP clients that authenticate with rotating bearer tokens: Microsoft Entra ID managed identities and service principals, typically obtained through
DefaultAzureCredential. No QuestDB credentials are stored.Today a static
token=is captured once. Once it expires, every reconnect presents the dead token. The existingHttpTokenProviderhook already re-pulls a token on every connect round. What's missing is an expiry-aware shared cache, failure classification, a refresh signal on 401, and a connect-string way to select a provider.Design documents
design/qwp-token-provider-spec.md: the cross-language specification (v0.6, all decisions resolved). Other clients (Rust/Python, Go, .NET, Node) implement this document.design/entra-id-qwp-auth.md: findings for the Java client (with code references), the design rationale, and the Java implementation plan (§10).Scope (design doc §10; test IDs refer to spec §10)
ExpiringToken,TokenSource,RefreshingTokenProvider,TokenUnavailableException. Tests C1–C8, C20.onTokenRejected. Tests C9–C16, C23.token_provider,azure_resource,azure_client_id,azure_credential), the shared registry, and thequestdb-client-azuremodule. Tests C17–C19, C24.execute(), instead of throwingQwpQueryClient not connectedfor the rest of its life.QwpQueryClientDynamicCredentialTest.testPooledQueryClientRecoversAfterAFailedFailoverreproduced the dead pooled worker before the fix.An earlier revision of this branch offered an opt-in
auth_failure_max_duration_millisdeadline that made an established sender terminal once authentication failures had lasted that long. Spec v0.6 withdraws it (§8.5, D7): it latched a terminal on a transient outage, and its clock kept running through the role-reject and transport rounds of a failover, so a failover between two brief credential failures could end a producer whose authentication never failed for long. Commit33200e93removes the connect-string key, the builder method and the send loop's outage clock.Separate ticket, not in this PR: treating a 503 at the WebSocket upgrade as transient in every phase.
Security-relevant server findings were reported privately (see
SECURITY.md) and are not part of this PR.Fixes
QueryWorker.shutdown()closed the query client once its bounded 5 s join expired, even with the dispatch thread still insideexecute(). A reconnect walk parked in a native connect or WebSocket upgrade does not see the shutdown interrupt, so it outlived the close: it went on to a healthy endpoint, then wrote the binds into the bind bufferclose()had freed (SIGSEGV), or published an I/O thread and connection on the closed client (leak). The failover reconnect could already reach this; egress recovery made it reachable from every query on such a client. When the join expires with the dispatch thread still busy, the thread now closes its own client on its way out ofrunLoop(), and one CAS decides which side closes.QueryWorkerBusyShutdownTestcovers it.executeImpldid not tell the health tracker, soQwpQueryClient.health()andQuestDB.health()kept reportingCONNECTED. Withfailover_max_attempts=1every later query fails on the same latched failure without reconnecting, so the client reportedCONNECTEDfor its whole life while each query failed. That exit now records the loss (RECONNECTING), and with a single attempt perexecute()it reportsFAILED.ConnectionHealthTestcovers both cases.PARSE_ERROR, reportedFAILEDwithoutageSincenone.ConnectionHealthdocuments none as "while connected", soQuestDB.health()left such senders out of its oldest outage.QwpConnectionHealthTracker.failed()now starts the outage when none is running; a client already without a connection keeps the start of the outage it is in.ConnectionHealthTest.testSenderHealthIsFailedWithAnOutageAfterATerminalRejectionfails without the fix (outageSince-1).handout_floorof its receipt is a failed fetch.RefreshingTokenProviderhands a token out only whilenow < expires_at - handout_floor(60 s by default), but a fetch whose token was already inside that window on arrival counted as a success. A source that issues tokens that short, such as a Keycloak realm at its 1-minute minimum access-token lifespan, left every caller waiting outcold_waitforno usable token from ... after waiting 30000 ms, with no cause, no WARN, and a provider that reported no failures.checkResult()now treats such a result as a retryable failed fetch whose message nameshandout_floor, so the WARN log,getLastFailure()and the caller's error all carry it. Spec v0.5 amended §4 and conformance rows C2 and C6 to match, and the README documents the floor andhandoutFloorMillis(...).RefreshingTokenProviderTest.testHandoutFloorTokenOnArrivalIsARetryableFailurepins the boundary (a 60 s token fails, 60.001 s is usable); reverting the check fails it and the updated C2 and C6 tests.azuremodule honours-Dtest.includeand-Dtest.exclude. Its surefire configuration mapped neither, so a questdb server build with thelocal-clientprofile, which pulls in the whole client reactor, ran all 26 Azure tests in every CI shard: about 35 s in each of the 15 self-hosted Linux shards of the OSS tandem's Azure DevOps build 279062. The module now maps both properties the waycoredoes, and its defaults still run every test. With-Dtest.include='**/griffin/**'the module runs no tests; before the fix the same command ran all 26 in 33 s.Tests
QwpQueryClientDynamicCredentialTest:connect()and a failover reconnect retry a refreshable 401 only once, even with a provider that hands out a new token on every pull. Without the limit, both walks kept retrying for as long as the server rejected.execute()makes after a failed failover fails as well, the operation reportsreconnect failed: ..., and a laterexecute()still reconnects instead of throwingQwpQueryClient not connected.testSyncInitializationRetryableProviderFailureExhaustsTheBudgetNamingTheClassfailed on the mac-aarch64 runner. It asserted more than two token pulls within a 300 ms connect budget. On that runner a timed backoff park wakes about 100 ms late (the sibling test's three parks took 196-504 ms on mac versus 67-72 ms on linux and windows), so the budget sometimes fit only two pulls. The test now uses a 2 s budget with unchanged assertions. Running the test JVM undertaskpolicy -breproduces the overshoot locally: three pulls in every run at 300 ms, one or two with added CPU load, and 15-17 at 2 s.testPersistent401AfterTheRetryFailsStartupfailed on the JDK 8 job (expected:<2> but was:<1>).TestWebSocketServercounted a validator rejection only after it had flushed the 401, so the test could readauthRejectCount()before the server thread incremented it; the CI log shows the client did make both attempts. The server now counts before it writes the response. With a 50 ms lag injected after every reject, the 40 tests in the three classes that read the counter pass; without the reorder the same lag fails that test every time.ConnectionHealthTest.testAuthenticationOutageNeverMakesTheSenderTerminaldrives credential-unavailable, 401 and role-reject rounds, checks that the sender stays alive, and checks that it delivers the buffered row after recovery. With a terminal latched on a credential failure in the send loop, it fails withexpected:<RECONNECTING> but was:<FAILED>.FAILEDhealth transitions. No test failed when either newfailed()report was deleted.testSenderHealthIsFailedWithAnOutageAfterATerminalRejectionnow pins the sender's, fromCursorWebSocketSendLoop.recordFatal(), andtestQueryClientHealthIsFailedWhenFailoverIsOffAndItLosesItsConnectionpins the query client's withfailover=off. Deleting the sender's report fails the first withexpected:<FAILED> but was:<CONNECTED>, and deleting the query client's fails the second withexpected:<FAILED> but was:<RECONNECTING>. The new query test shares a helper with thefailover_max_attempts=1test.testForcedRefreshIsRateLimitedIgnoresStaleTokensAndAcceptsTheSameTokenpresented the stale token withinforced_min_intervalof the forced refresh, so the rate limit returned first: with the stale-token check deleted, that test and the other credential test classes still passed. The step now runs once the interval has passed and asserts that no fetch is scheduled; deleting the check fails it.RefreshingTokenProviderTest. The JDK 8 CI job failed with aNullPointerExceptionin the condition of anawait()intestForcedRefreshIsRateLimitedIgnoresStaleTokensAndAcceptsTheSameToken. Fourawait()conditions readManualScheduler.peek()twice while another thread made the provider replace its scheduled task. The provider cancels the old task before it schedules the new one, andpeek()returns live tasks only, so the second read could find no task.ManualScheduler.isNextImmediate()now reads the task list once, and the four awaits use it. With pauses added between the cancel and the schedule, the old condition failed 3 of 3 runs with the CI stack trace and the new one passed 3 of 3. The race dates from the original test, not from the review follow-up.Docs
Sender,HttpTokenProvider,QwpCredentialUnavailableException,QwpWebSocketSender,CursorWebSocketSendLoopandQwpQueryClientstill said that an eager initial connect fails fast with the provider's own exception. They now describe the SYNC retry of a retryable provider failure, theLineSenderExceptionthat reports an exhausted budget, the extra token pull after a 401, and an established sender that retries every credential failure indefinitely and reports each one to its error handler and connection health. The merge ofmainhad also re-added, at thebuild()callsite, the recovery comment that feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap #91 moved intoconstructEngineOnSlotLocked, and dropped two feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap #91 comments;main's text is restored. No behaviour changes.Tradeoffs and behaviour changes
Sender.health(). An application that prefers a hard failure closes the sender itself; withoutsf_dir, the unacknowledged rows are lost when it closes.HttpTokenProvideron QWP now gets one extragetToken()call after a refreshable 401, and one same-endpoint retry when the token changed.forced_wait(5 s) pluscold_wait(30 s) while it holds the foreground connect-walk lock.close()breaks the wait with an interrupt.QueryWorker.shutdown()leaves the client of anexecute()that never returns open, together with its token-provider lease, instead of freeing memory that call still uses.QwpAuthFailedExceptionmessages start withauth-rejected:; query-client provider failures arrive asQwpCredentialUnavailableExceptionwith acredential-unavailable:prefix;auth failure during failover reconnectis nowauth-rejected during failover reconnect. The OSS tandem updates the one test that matched the old wording.lazy_connect=trueas well.handout_floornow records failed fetches, logged at WARN at most once a minute, where the provider previously counted them as successes it could not hand out. Since spec v0.5, this is the rule for every client.local-clientprofile) resolve the Azure SDK dependencies and compile theazuremodule; its tests run only in shards whose-Dtest.includeselects them.token_provideror the 401 refresh retry. The bytes on the wire are unchanged, the enterprise server answers a rejected credential with 401 (pinned by itsQwpWebSocketAclTest), and the loopback tests cover a 401 with no challenge and with aBasicchallenge.OSS test-compile compatibility
Client
mainadded symbol-dictionary recycle in #91 after this branch diverged. The OSS tandem questdb/questdb#7799 includes tests that call that API, so its previous client pin (6048875f) could not compile them. Commit96fd9444merges clientmainhere and retains the rotating-credential handling while wiring the recycle config and rebuild factory into the sender builder. JDK 25 client compilation and 338 targeted tests passed (one skipped), and the OSS and enterprise test sources compiled against it.