feat(csharp): Add retry-after behavior for 503 responses in Spark ADBC driver #2664
Conversation
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
CurtHagenlocher
requested changes
Apr 3, 2025
Contributor
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks for the change! I've left feedback, some more optional than others.
Contributor
|
Can we hold off submitting this until a decision is made on #2672? |
jadewang-db
requested review from
kou,
lidavidm,
wjones127 and
zeroshade
as code owners
jadewang-db
force-pushed
the
handle-retry-after
branch
2 times, most recently
from
6906845 to
0b4526b
Compare
CurtHagenlocher
requested changes
Apr 16, 2025
Contributor
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks for the change! Please make sure the linter doesn't complain about whitespace issues and that the retry flag is properly handled.
Contributor
|
There are some merge conflicts, so this will probably need to be rebased. |
Update DatabricksConnection.cs Update RetryHttpHandlerTest.cs Add retry after handling in ADBC Spark driver fix pre commit check failures fix build error address PR comments fix linter lint Update DatabricksConnection.cs address comments
jadewang-db
force-pushed
the
handle-retry-after
branch
from
9acda68 to
b89d9be
Compare
colin-rogers-dbt
pushed a commit
to dbt-labs/arrow-adbc
that referenced
this pull request
Jun 10, 2025
…C driver (apache#2664) ## Description This PR implements retry-after behavior for the Spark ADBC driver when receiving 503 responses with Retry-After headers. This is particularly useful for Databricks clusters that may return 503 responses when a cluster is starting up or experiencing temporary unavailability. ## Changes - Added new configuration parameters: - `adbc.spark.temporarily_unavailable_retry` (default: 1 - enabled) - `adbc.spark.temporarily_unavailable_retry_timeout` (default: 900 seconds) - Created a `RetryHttpHandler` class that wraps the existing `HttpClientHandler` to handle 503 responses - Modified `SparkHttpConnection` to use the new retry handler - Added comprehensive unit tests for the retry behavior ## Implementation Details When a 503 response with a Retry-After header is received: 1. The handler will wait for the number of seconds specified in the header 2. It will then retry the request 3. If another 503 response is received, it will continue retrying 4. If the total retry time exceeds the configured timeout, it will fail with an appropriate error message ## Testing Added unit tests to verify: - Retry behavior for 503 responses with Retry-After headers - Timeout behavior when retry time exceeds the configured limit - Handling of invalid or missing Retry-After headers - Disabling retry behavior via configuration - Parameter validationv
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.
Description
This PR implements retry-after behavior for the Spark ADBC driver when receiving 503 responses with Retry-After headers. This is particularly useful for Databricks clusters that may return 503 responses when a cluster is starting up or experiencing temporary unavailability.
Changes
adbc.spark.temporarily_unavailable_retry(default: 1 - enabled)adbc.spark.temporarily_unavailable_retry_timeout(default: 900 seconds)RetryHttpHandlerclass that wraps the existingHttpClientHandlerto handle 503 responsesSparkHttpConnectionto use the new retry handlerImplementation Details
When a 503 response with a Retry-After header is received:
Testing
Added unit tests to verify: