[
https://issues.apache.org/jira/browse/CASSSIDECAR-465?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arjun Ashok updated CASSSIDECAR-465:
------------------------------------
Authors: Arjun Ashok
Test and Documentation Plan:
Test Plan
- RequestExecutorTest (new): regression test locking in the schedule() fix —
verifies the runnable fires exactly once: immediately when delayMillis <= 0,
only after the delay when delayMillis > 0 (the prior bug fired both).
- BasicRetryPolicyTest (updated): new 429 coverage mirroring the existing 503
tests — retries with backoff, honors a valid Retry-After header, falls back to
configured delay on an invalid one, respects maxRetries; also updated the
generic 4xx-error sweep test to exclude 429 now that it has dedicated handling.
- SSTableUploadConfigurationImplTest (updated): default value of
retryAfterSeconds(), and a regression test confirming the deprecated 3-arg
constructor still defaults it correctly.
- SSTableUploadHandlerTest (updated): Retry-After header is present with the
correct value on the 429 response when the concurrent upload limit is exceeded.
- Full ./gradlew :client:test and ./gradlew :server:test — both green (aside
from a pre-existing, unrelated LiveMigrationFileStreamTest failure caused by
loopback aliases not configured on the test machine, isolated and confirmed
unrelated to this change).
- Backward-compatibility check: verified empirically that an existing
sidecar.yaml missing the new retry_after_seconds key still deserializes
correctly and defaults to 1, so no operator action is needed on upgrade.
Documentation
- docs/src/user.adoc: added retry_after_seconds to the sstable_upload config
section documentation.
- conf/sidecar.yaml: added the new property (with inline comment) to the
canonical sample config.
- CHANGES.txt: entry added (and revised per review feedback to name both fixes
explicitly, not just the 429-facing one).
- Worth a short Compatibility note too, since it came up in review:
retryAfterSeconds() is a default interface method and the old 3-arg
SSTableUploadConfigurationImpl constructor is preserved as deprecated — both
additions to SSTableUploadConfiguration/Impl are non-breaking for any existing
implementer/caller of the published server artifact.
Status: Patch Available (was: Open)
> Improve HTTP 429 handling across sidecar server and client
> ----------------------------------------------------------
>
> Key: CASSSIDECAR-465
> URL: https://issues.apache.org/jira/browse/CASSSIDECAR-465
> Project: Sidecar for Apache Cassandra
> Issue Type: Bug
> Components: Bulk Analytics, Client
> Reporter: Arjun Ashok
> Assignee: Arjun Ashok
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> *Update (see comments):* _fixing this requires also fixing a bug in
> {{RequestExecutor.schedule()}} that causes every delayed retry (including the
> existing 503 {{Retry-After}} path) to fire immediately in addition to firing
> after the delay. This is a prerequisite for this ticket's fix; see comment
> for details._
> The sidecar server returns HTTP 429 in two cases: when the concurrent upload
> limit is exceeded (SSTable uploads) and when the stream request rate limit is
> exhausted (SSTable component streaming). In both cases the client has no
> meaningful retry behavior for this response.
> BasicRetryPolicy, which all retry policy implementations extend, handles 503
> SERVICE_UNAVAILABLE specially by parsing the Retry-After response header and
> applying backoff before retrying. HTTP 429 falls into the generic 4xx
> catch-all instead, which either retries immediately on a different host with
> no backoff, likely hitting the same limit again or fails permanently if no
> alternative host is available.
> Gap on server side: SSTableUploadHandler does not include a Retry-After
> header in its 429 response, unlike the streaming path which does. This means
> even with a client-side fix, upload callers cannot benefit from
> server-indicated retry timing.
> Expected behavior:
> BasicRetryPolicy should handle 429 analogously to 503, parsing Retry-After if
> present and otherwise applying exponential backoff before retrying. Since all
> retry policies extend BasicRetryPolicy, this fix applies uniformly across all
> APIs. Additionally, SSTableUploadHandler should include a Retry-After header
> in its 429 response so the client can back off for an appropriate duration.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
