Client config reporting — stage 2: full DRIVER_CONFIG report#968
Draft
nikagra wants to merge 5 commits into
Draft
Client config reporting — stage 2: full DRIVER_CONFIG report#968nikagra wants to merge 5 commits into
nikagra wants to merge 5 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…plumbing)
Stage 1 of driver configuration reporting: the mechanism to report a client's
effective configuration to ScyllaDB at connection time, so operators can inspect
driver settings (via system.clients.client_options) while investigating incidents.
Gated behind a new option, advanced.client-config-reporting.enabled, which
ships disabled — when off there is zero change on the wire. When enabled:
- SESSION_ID: a dedicated, driver-generated per-session UUID, sent on every
connection (control and pool) so the server can group a session's
connections. Independent of the user-settable CLIENT_ID.
- DRIVER_CONFIG: a compact JSON blob, sent only on the control connection.
Stage 1 emits only {"version":1} (version metadata only); the full
configuration report follows in stage 2.
Both entries are produced by a new DriverConfigReporter, invoked per connection
from ProtocolInitHandler and evaluated fresh (so toggling the flag at runtime
takes effect on new connections). Reporting is fail-safe: any failure while
building the report is swallowed and never breaks connection initialization.
The config option is registered consistently across DefaultDriverOption,
TypedDriverOption, OptionsMap.driverDefaults() and reference.conf, and is
covered by a Simulacron IT asserting the STARTUP payload.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fills in DefaultDriverConfigReporter with all configuration groups of the
cross-driver schema, replacing the stage-1 {"version":1} placeholder:
- connection, socket, control_pane_settings: mapped from the default profile
(timeouts in ms; keys with no Java equivalent — read/write timeout,
server-side internal-query timeout — reported as null).
- reconnection_policy / retry_policy / speculative_execution_policy: a
discriminated object chosen by the configured policy class, with a "custom"
fallback carrying the class name for user policies.
- load_balancing_policy: normalized flags for DefaultLoadBalancingPolicy
(token-aware, dc-failover, explicit/inferred local dc, ...), "custom"
otherwise.
- connection_pool, query_defaults, tls: pool sizing / shard-aware port,
per-request defaults (page size, consistency, idempotence, client
timestamps), and TLS booleans only (never credentials or hosts).
Values are read from the default execution profile and the context's policy
accessors on each control-connection init, so the report reflects the current
(possibly reloaded) configuration. Building remains fail-safe.
Adds unit coverage for the default report shape and each policy discrimination
branch, driven by OptionsMap.driverDefaults().
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ClientConfigReportingCcmIT is the real-server counterpart of the Simulacron test: it starts ScyllaDB via CCM, connects with client config reporting enabled, and reads back system.clients.client_options to assert what the server actually stored: - SESSION_ID is present on every one of the session's connections, with a single shared value; - DRIVER_CONFIG is stored for exactly one connection (the control connection), identified independently as the connection carrying it. Verified against ScyllaDB 2026.1.9: the server accepts the extra STARTUP keys and stores the full DRIVER_CONFIG report intact (no length truncation), and transient protocol-version negotiation attempts leave no rows in system.clients. Scylla-only via @BackendRequirement; runs in the parallelizable CCM suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nikagra
force-pushed
the
feature/driver-config-reporting-phase2
branch
from
July 23, 2026 16:43
9335dac to
7c6eb1a
Compare
ClientConfigReportingCcmIT reads system.clients.client_options, a column that only exists on newer ScyllaDB releases. On the LTS-PRIOR lane (Scylla 2025.1.14) the column is absent, so the SELECT fails with InvalidQueryException: "Unrecognized name client_options", failing the "Scylla ITs (LTS-PRIOR, 11, parallelizable)" check (the test carries @category(ParallelizableTests.class), so only that lane runs it). Add a minInclusive="2026.1.0" bound to the existing @BackendRequirement so the test is skipped on versions without the column and still runs on 2026.1+ (LTS-LATEST 2026.1.9 and LATEST already pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ER_CONFIG report
Code review of the stage-2 (full report) work surfaced several correctness bugs:
- node-location-preference ignored SessionBuilder.withLocalDatacenter(), only
checking the config option, and silently dropped an explicitly-configured
rack when no DC was set (now reported as rack-auto instead of being lost).
- Policy discrimination used instanceof against non-final built-in classes,
so DcInferringLoadBalancingPolicy (a real, documented policy) was
misreported as "default", and any user subclass of a built-in
reconnection/retry/speculative-execution/load-balancing policy was
misreported as the unmodified built-in instead of "custom". Switched all
four to exact-class checks; DcInferringLoadBalancingPolicy now gets its own
"dc-inferring" type.
- customPolicy() reported an empty name for anonymous policy classes; falls
back to the binary class name now.
- CLIENT_CONFIG_REPORTING_ENABLED's javadoc/reference.conf comment still
described the stage-1 {"version":1}-only placeholder.
- Corrected a stale rationale comment in ClientConfigReportingSimulacronIT.
Added test coverage for each fix, and documented (rather than worked around)
remaining known limitations: single-execution-profile scoping and
LazyReference init-ordering assumptions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
What
Stage 2 of driver configuration reporting: fills in the full
DRIVER_CONFIGJSON report, replacing the stage-1{"version":1}placeholder. Whenadvanced.client-config-reporting.enabled = true, the control connection now reports the driver's effective configuration as the cross-driver schema shape.The report
All 11 groups are populated from the default execution profile and the context's policies, rebuilt on each control-connection init (so it reflects the current, possibly reloaded, config):
USING TIMEOUT) are reported asnull.exponential/constant,standard-error-aware/downgrading-consistency,constant/none), with acustomfallback carrying the user policy's class name.DefaultLoadBalancingPolicy(token_aware,dc_failover, explicit-vs-inferredlocal_dc,local_rack, ...);customotherwise.Reporting remains fail-safe: any failure while building the report is swallowed;
SESSION_IDis still emitted and only the config blob is dropped.Reconciliation decisions (Java ↔ schema)
load_balancing_policy.shuffle→false(Java has no replica-shuffle option).control_pane_settings.system_query_server_side_timeout_ms→null(Java's metadata timeout is client-side; it does not emitUSING TIMEOUT).{type:"custom", name:<class simple name>}only (no reflective attribute dump in v1).Testing
DefaultDriverConfigReporterTestexpanded to 17 tests: the default report shape (golden), each policy discrimination branch, explicit local-dc, server-side timestamps, TLS enabled, socket overrides, unbounded page size, plus the stage-1 gating/fail-safe cases. Report content is asserted against a real config fromOptionsMap.driverDefaults().ProtocolInitHandlerTestgreen.Follow-up
SELECT client_options['DRIVER_CONFIG'], client_options['SESSION_ID'] FROM system.clients, assert the JSON is schema-valid andSESSION_IDis identical across a session's rows. (Needs a running cluster; not included here.)STARTUPkey tolerance +client_optionsvalue-length).🤖 Generated with Claude Code