Allow retry policies to handle client-side request timeouts#959
Closed
dkropachev wants to merge 2333 commits into
Closed
Allow retry policies to handle client-side request timeouts#959dkropachev wants to merge 2333 commits into
dkropachev wants to merge 2333 commits into
Conversation
Bumps [sphinx-scylladb-theme](https://github.com/scylladb/sphinx-scylladb-theme) from 1.8.6 to 1.8.7. - [Release notes](https://github.com/scylladb/sphinx-scylladb-theme/releases) - [Commits](scylladb/sphinx-scylladb-theme@1.8.6...1.8.7) --- updated-dependencies: - dependency-name: sphinx-scylladb-theme dependency-version: 1.8.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Go over disabled tests and enable them. If test fails: 1. Make test framework ignore it with proper message 2. If it is scylla bug or test needs to be adjusted, create an issue and reference it 3. Fix test to match scylla behavior There are two ways to disable test for scylla: 1. Mark it with @ScyllaSkip 2. Mark it with any backend requirement annotation: @CassandraRequirement, @DseRequirement, @BackendRequirement; and do not add sylla-specific requirement.
Name does not reflect logic. This annotation makes test framework to skip all non-scylla backends.
Removes DropwizardMetricsIT, MicroProfileMetricsIT, MicrometerMetricsIT from parallelizable tests category, making them serial. Additionally enables MicrometerMetricsIT which was previously ignored due to flakiness. `should_evict_down_node_metrics_when_timeout_fires` in MetricsITBase relies on setting static field `MIN_EXPIRE_AFTER` in AbstractMetricUpdater and then setting it back to 5 minutes. Those three test classes inherit this test method and when they run concurrently it sometimes results in setting the minimal expire time back to 5 minutes before another test initializes its session with expire time set to 1 second. This can be evidenced by the appearance of lines like ``` c.d.o.d.i.c.m.AbstractMetricUpdater - [s6] Value too low for advanced.metrics.node.expire-after: PT1S. Forcing to PT5M instead. ``` whenever that method fails. It does not appear when the test passes (due to lucky schedule). Making those tests serial seems to be the simplest solution.
Bumps [sphinx-scylladb-theme](https://github.com/scylladb/sphinx-scylladb-theme) from 1.8.7 to 1.8.8. - [Release notes](https://github.com/scylladb/sphinx-scylladb-theme/releases) - [Commits](scylladb/sphinx-scylladb-theme@1.8.7...1.8.8) --- updated-dependencies: - dependency-name: sphinx-scylladb-theme dependency-version: 1.8.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Adds isolated SessionTicketsIT which checks if session tickets mechanism behaves as expected. It does that by watching Java's SSL debug logs and ensuring that specific substrings appear in them. Since the server supports tickets with TLSv1.3, only that version is tested. In that version the session resumption without server-side state is done through pre-shared keys. The details are described in rfc8446 (see section 2.2).
When HdrHistogram writes latency if latency is higher than maximum expected it throws error. Driver catches this error and logs it. Users get confused since there is no reciepe how to fix it. Solution is to set advanced.metrics.session.cql-requests.highest-latency to maximum possible latency, which is request timeout. But since request timeout could be configured on the execution profile, we can't automatically tune highest-latency to match it. So, only good solution is to give user a hint in the log message.
Adds a new option that allows for throttling `addMissingChannels()` in `ChannelPool.java`. Instead of creating all missing channels at once for particular channel pool the driver will create batches of limited size and handle them sequentially. Setting size 0 means allowing unlimited batches and it will result in the usual behavior of creating channels all at once. The default value is 0. Setting any other size `N`, will result in driver limiting the batches to either `N`, or `ceil(target_total_number_of_pool_connections / 2)`, whichever is smaller. The main motivation for this change is to allow for leveraging TLSv1.3 stateless session resumption using session tickets. Most of the time driver ends up without usable tickets for connecting, mainly due to the way the management of those tickets is done by Java itself. By running connection in batches it allows the driver to obtain some tickets for the next batches, allowing at least some of the connections to avoid negotiation. This feature can also be used outside of this situation. Just throttling should also bring some relief in case of mass reconnections.
Modifies current implementation of `addMissingChannels` in `ChannelPool` to make use of the newly added option. Default behavior remains unchanged.
Adds a method to get contact points with default shard aware port from `ccmBridge`.
Adds `all_reconnections_but_one_should_use_tickets_when_throttled_TLSv13` to `SessionTicketsIT`. This test checks whether throttling reconnections to a batch size of 1 helps with the session resumptions. Since the Java 11 cache can only store information for 1 session resumption this in theory should allow for resumptions to happen if the connections are established sequentially. The test relies on server to send NewSessionTicket messages in a timely manner.
Implement class to store information about all features negotiated.
No need to pull this information all the time from the channel. Let's cache it when it is populated to the channel and use it.
…ven-plugin to v1.11
…y-plugin to v3.7.1
Raise the supported runtime and build floor to Java 11, and align docs, configuration, formatting, and dependency versions with that baseline.
Reject invalid subnet prefix strings so address translation fails clearly instead of accepting bad configuration.
Update maintained CI paths for the Java 11 baseline, including JDK 17 compiler flags, javadoc skips, authenticated version lookup, examples compilation, and runtime-specific reports.
Enable required annotation processors, silence Java 11 compiler warnings, and improve CCM cleanup logging so integration failures are easier to diagnose. Update tests for newer Mockito behavior by avoiding shared MockitoJUnitRunner usage in parallel integration tests and by verifying cache interactions explicitly where the mock behavior changed. Fix CI failures exposed by the updated validation matrix: provide Micrometer histogram bounds in the no-SLA test, register config-change listeners before manual reloads, avoid assuming a transient one-connection node state before reconnection wins the race, and validate prepared-statement invalidation through observable cache behavior instead of a test-only removal-listener latch.
Keep PRIMITIVE_CODECS public for source and binary compatibility, deprecate direct array access, and add getPrimitiveCodecs() as the immutable-list accessor. Refs scylladb#908
The reusable workflow main_pr_events_jira_sync.yml in scylladb/github-automation used actions/checkout@v4 (a tag ref), which violates the scylladb org policy requiring all actions to be pinned to full commit SHAs. This caused every PR targeting scylla-4.x to fail with: Error: The action actions/checkout@v4 is not allowed in scylladb/java-driver because all actions must be pinned to a full-length commit SHA. Update the pinned SHA to ef3a6238c30003cbab9a339c32e5112f6889a197 which pins actions/checkout to 34e114876b0b11c390a56381ad16ebd13914f8d5 (v4.3.1). Fixes: scylladb#911 Upstream fix: scylladb/github-automation#192
…kout" (scylladb#913) This reverts commit e280db0.
…2026-45416 netty-handler prior to 4.1.135.Final is affected by two HIGH severity vulnerabilities: - CVE-2026-44249 (CVSS 8.1): IpSubnetFilterRule.compareTo() performs an incorrect masking operation, allowing attackers to bypass IPv6 subnet ACL rules with valid public IP addresses. - CVE-2026-45416 (CVSS 7.5): SslClientHelloHandler.decode() eagerly allocates up to 16 MiB of unpooled memory when maxClientHelloLength=0 (the SniHandler default). A crafted TLS ClientHello can trigger memory exhaustion (DoS). One-line change in the root pom.xml. All consumers of java-driver-core will inherit the fix transitively once a new driver release is published. Tracked in: scylladb/kafka-connect-scylladb#184
Fixes scylladb/scylladb-docs-homepage#204 Straightforward fixes (typos / moved pages): - faq/README.md, upgrade_guide/README.md: loadbalancing -> load_balancing path - manual/core/non_blocking/README.md: remove trailing backtick from SyncCqlSession URL - manual/developer/common/concurrency/README.md: same trailing backtick fix - manual/core/reactive/README.md: remove trailing ] from DataStax devapp URL - manual/core/statements/batch/README.md: http -> https; replace dead DSE 6.7 batch URL with canonical Cassandra CQL DML docs (#batch_statement anchor) - manual/core/query_timestamps/README.md: replace dead DSE 6.0 LWT URL with canonical Cassandra CQL DML docs - manual/mapper/entities/README.md: fix malformed ?# anchor and update to current Cassandra DML docs URL - manual/mapper/daos/queryprovider/README.md: add missing /entity/ path segment in EntityHelper Javadoc URL Opinionated fixes (replacement content chosen): - README.md: replace defunct maven-badges.herokuapp.com badge with shields.io pointing to central.sonatype.com - manual/core/load_balancing/README.md: replace dead DataStax fault-tolerance whitepaper PDF with YouTube video (https://www.youtube.com/watch?v=NT2-i3u5wo0) - manual/core/graalvm/README.md: update GraalVM native-image resources URL to current path; drop hyperlinks from DSE geotypes and DSE Graph entries (pages do not exist in the Scylla driver docs) - manual/core/metrics/README.md: update Micrometer JMX registry URL to current docs.micrometer.io path - manual/developer/netty_pipeline/README.md: rename '### InFlightHandler' heading to '### In-Flight Handler' to match the existing anchor reference in request_execution/README.md Verified clean (for these changes) via local `make -C docs linkcheck` run.
When the initial TCP connection to a node fails, ChannelPool.connectFuture completes (so the pool is added to PoolManager.pools) while `channels` is still null — initialize() is only called on the success path. The four methods size(), getAvailableIds(), getInFlight(), and getOrphanedIds() called Arrays.stream(channels) without guarding against this, throwing NullPointerException whenever a Dropwizard Metrics reporter (JMX, Graphite, etc.) scraped the gauge values during the reconnection window. Fix: mirror the pattern already used by next(), which checks singleThreaded.initialized before touching channels. Using the volatile initialized flag is correct by the JMM: the volatile write at line 365 (initialized = true) happens-after all channels writes, so any thread that reads initialized == true is guaranteed to see channels as fully populated. Fixes: CUSTOMER-413
If the initial pool connection attempt fails, ChannelPool completes while still uninitialized and starts a reconnect loop. A session close during that state can complete immediately because there are no active channels yet. When the pending reconnect later succeeds, the uninitialized reconnect path previously initialized the pool and kept the new channel open even though the pool had already been closed. This could leave a leaked channel and incorrectly publish channelOpened after shutdown. Guard that reconnect-success path with isClosing: force-close the new channel and complete the reconnect attempt without initializing the pool. Add coverage for both closeAsync and forceCloseAsync when the first successful channel arrives after shutdown.
Apply the sphinx-scylladb-theme update from PR scylladb#869 as a focused docs dependency bump. PR scylladb#869 carries unrelated repository-wide changes on top of the dependency update, so this commit keeps the branch limited to the docs dependency metadata only. Changes included: - raise the docs pyproject dependency lower bound from sphinx-scylladb-theme >=1.9.1 to >=1.9.2 - refresh docs/uv.lock to pin sphinx-scylladb-theme 1.9.2 with the matching sdist and wheel hashes
maven-release-plugin 3.3.1 (released May 2026) breaks the GitHub release flow with "preparation step was stopped mid-way". The root cause is an SCM API incompatibility: the pom.xml pins maven-scm-provider-gitexe:1.9.5 (SCM 1.x), while plugin 3.x requires the SCM 2.x API internally. Reverting to 2.5.3 as a workaround — it pairs correctly with the pinned scm-provider-gitexe 1.9.5. See scylladb#935 for the proper upgrade path.
…ckson 2.21 The top-level <configuration><skip>true</skip></configuration> block was introduced in deaeb1c ("build: drop Java 8 support", May 25 2026) to paper over a javadoc compilation failure caused by the Netty 4.1.135.Final bump. That version added compile-time references to org.jetbrains:annotations (@NotNull, @VisibleForTesting, @Async.Execute/Schedule). Jackson 2.21.x sources also reference ch.randelshofer:fastdoubleparser, which is bundled- and-shaded inside jackson-core with no standalone resolvable Maven coordinate. With <skip>true</skip> at the plugin level, no javadoc jar is attached for the core-shaded artifact, causing Sonatype Central to reject the release. Fix: - Remove the global skip flag (the attach-javadocs execution is still in place) - Add org.jetbrains:annotations:23.0.0 to additionalDependencies (matches the version Netty uses as a provided dependency) - Set failOnError=false for the fastdoubleparser case; those sources are already excluded from the output via excludePackageNames, so the jar still contains the full public API documentation
Update docs actions to the latest version. ## How to test See **Docs / Build PR** workflow (`github/workflows/docs-pr.yaml`), it should not raise this warning: <img width="915" height="181" alt="image" src="https://github.com/user-attachments/assets/fc3faf58-2c19-4a80-b3a5-c14b625a0d96" />
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe driver adds Sequence Diagram(s)sequenceDiagram
participant Client
participant RequestHandler
participant RetryPolicy
participant Metrics
Client->>RequestHandler: execute request
RequestHandler->>RetryPolicy: onRequestTimeout(...)
RetryPolicy-->>RequestHandler: RetryDecision
RequestHandler->>Metrics: record timeout outcome
RequestHandler-->>Client: retry request or raise timeout
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@driver-core/src/main/java/com/datastax/driver/core/policies/IdempotenceAwareRetryPolicy.java`:
- Around line 91-104: Update IdempotenceAwareRetryPolicy.onRequestTimeout to
determine idempotency by calling isIdempotent(statement) instead of relying on
the statementIdempotent argument, preserving the existing childPolicy delegation
for idempotent statements and rethrow behavior otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e45c3bd8-5552-4cf2-9517-21886be81744
📒 Files selected for processing (17)
changelog/README.mdclirr-ignores.xmldriver-core/src/main/java/com/datastax/driver/core/RequestHandler.javadriver-core/src/main/java/com/datastax/driver/core/policies/DefaultRetryPolicy.javadriver-core/src/main/java/com/datastax/driver/core/policies/DowngradingConsistencyRetryPolicy.javadriver-core/src/main/java/com/datastax/driver/core/policies/FallthroughRetryPolicy.javadriver-core/src/main/java/com/datastax/driver/core/policies/IdempotenceAwareRetryPolicy.javadriver-core/src/main/java/com/datastax/driver/core/policies/LoggingRetryPolicy.javadriver-core/src/main/java/com/datastax/driver/core/policies/RetryPolicy.javadriver-core/src/test/java/com/datastax/driver/core/policies/AbstractRetryPolicyIntegrationTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/CustomRetryPolicyIntegrationTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/DefaultRetryPolicyIntegrationTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/DowngradingConsistencyRetryPolicyIntegrationTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/FallthroughRetryPolicyIntegrationTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/IdempotenceAwareRetryPolicyIntegrationTest.javamanual/retries/README.mdmanual/socket_options/README.md
dkropachev
force-pushed
the
dk/issue-958-request-timeout-retry
branch
from
July 15, 2026 14:04
83d3721 to
dff163d
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new retry-policy callback to handle client-side request timeouts (represented by OperationTimedOutException due to the driver read timeout), allowing policies to decide whether to retry based on resolved statement idempotence and retry count, while keeping default behavior conservative (rethrow).
Changes:
- Adds
RetryPolicy.onRequestTimeout(...)and routes client-side timeout handling through it fromRequestHandler. - Updates built-in retry policies and logging wrapper to support the new callback; adjusts default behavior to rethrow client-side timeouts.
- Updates integration tests and manual docs/changelog/Clirr ignore to reflect the new API and behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| manual/socket_options/README.md | Docs update to describe timeout behavior and new retry-policy hook. |
| manual/retries/README.md | Docs update adding onRequestTimeout section and idempotence guidance. |
| driver-core/src/test/java/com/datastax/driver/core/policies/IdempotenceAwareRetryPolicyIntegrationTest.java | Updates assertions and adds policy override for request timeouts. |
| driver-core/src/test/java/com/datastax/driver/core/policies/FallthroughRetryPolicyIntegrationTest.java | Verifies fallthrough behavior via onRequestTimeout. |
| driver-core/src/test/java/com/datastax/driver/core/policies/DowngradingConsistencyRetryPolicyIntegrationTest.java | Updates expectations to use onRequestTimeout. |
| driver-core/src/test/java/com/datastax/driver/core/policies/DefaultRetryPolicyIntegrationTest.java | Aligns test to new default behavior (rethrow on client timeout). |
| driver-core/src/test/java/com/datastax/driver/core/policies/CustomRetryPolicyIntegrationTest.java | Adds custom retry-on-timeout behavior coverage. |
| driver-core/src/test/java/com/datastax/driver/core/policies/AbstractRetryPolicyIntegrationTest.java | Adds Mockito verifiers for onRequestTimeout invocations. |
| driver-core/src/main/java/com/datastax/driver/core/RequestHandler.java | Routes client timeout decision through computeRetryDecisionOnRequestTimeout. |
| driver-core/src/main/java/com/datastax/driver/core/policies/RetryPolicy.java | Adds default onRequestTimeout API with idempotence + retry count context. |
| driver-core/src/main/java/com/datastax/driver/core/policies/LoggingRetryPolicy.java | Logs decisions for request timeouts. |
| driver-core/src/main/java/com/datastax/driver/core/policies/IdempotenceAwareRetryPolicy.java | Applies idempotence gating to request-timeout retries. |
| driver-core/src/main/java/com/datastax/driver/core/policies/FallthroughRetryPolicy.java | Explicitly rethrows on request timeouts. |
| driver-core/src/main/java/com/datastax/driver/core/policies/DowngradingConsistencyRetryPolicy.java | Retries request timeouts on next host for idempotent statements. |
| driver-core/src/main/java/com/datastax/driver/core/policies/DefaultRetryPolicy.java | Changes default policy to rethrow client-side request timeouts. |
| clirr-ignores.xml | Suppresses Clirr difference for the new interface method. |
| changelog/README.md | Changelog entry for #958 behavior/API improvement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [onReadTimeout]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/RetryPolicy.html#onReadTimeout-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-int-int-boolean-int- | ||
| [onRequestError]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/RetryPolicy.html#onRequestError-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-com.datastax.driver.core.exceptions.DriverException-int- | ||
| [OperationTimedOutException]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/exceptions/OperationTimedOutException.html No newline at end of file | ||
| [onRequestTimeout]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/RetryPolicy.html#onRequestTimeout-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-com.datastax.driver.core.exceptions.OperationTimedOutException-boolean-int- |
| [onReadTimeout]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/DefaultRetryPolicy.html#onReadTimeout-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-int-int-boolean-int- | ||
| [onWriteTimeout]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/DefaultRetryPolicy.html#onWriteTimeout-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-com.datastax.driver.core.WriteType-int-int-int- | ||
| [onUnavailable]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/DefaultRetryPolicy.html#onUnavailable-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-int-int-int- | ||
| [onRequestTimeout]: https://docs.datastax.com/en/drivers/java/3.11/com/datastax/driver/core/policies/RetryPolicy.html#onRequestTimeout-com.datastax.driver.core.Statement-com.datastax.driver.core.ConsistencyLevel-com.datastax.driver.core.exceptions.OperationTimedOutException-boolean-int- |
dkropachev
force-pushed
the
dk/issue-958-request-timeout-retry
branch
from
July 16, 2026 11:33
dff163d to
0080f1c
Compare
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.
Fixes #958
Summary
Testing