Skip to content

Commit e08f941

Browse files
Add DATABRICKS_DISABLE_RETRIES config option to disable the default retry mechanism. (#523)
## What changes are proposed in this pull request? This PR adds a config-level property to disable the default retry mechanism. That is, no request will be retried no matter its error code/status. This is implemented via a new retry strategy `NoRetryStrategy` which systematically returns `false`. This PR addresses some of the limitations called out in issue: #289. **Why not providing control on the number of retries directly?** The number of retries should be considered an implementation detail of a specific retry strategy. The Databricks SDKs are meant to evolve to a model where the retry conditions are more and more server controlled (e.g. leverage the RetryInfo error details). Rather than providing control over the number of retries, we intent to provide users with control over (i) the overall timeout of the method call (including retries), and (ii) the retry strategy used on a specific client. ## How is this tested? Unit + Integration tests.
1 parent dbb5ed2 commit e08f941

File tree

7 files changed

+112
-1
lines changed

7 files changed

+112
-1
lines changed

NEXT_CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
### New Features and Improvements
66

7+
* Add a new config attribute `DATABRICKS_DISABLE_RETRIES` to disable the
8+
default retry mechanism.
9+
710
### Bug Fixes
811

912
### Documentation

databricks-sdk-java/src/main/java/com/databricks/sdk/core/ApiClient.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import com.databricks.sdk.core.http.Request;
77
import com.databricks.sdk.core.http.RequestOptions;
88
import com.databricks.sdk.core.http.Response;
9+
import com.databricks.sdk.core.retry.NoRetryStrategyPicker;
910
import com.databricks.sdk.core.retry.RequestBasedRetryStrategyPicker;
1011
import com.databricks.sdk.core.retry.RetryStrategy;
1112
import com.databricks.sdk.core.retry.RetryStrategyPicker;
@@ -49,8 +50,14 @@ public Builder withDatabricksConfig(DatabricksConfig config) {
4950
this.httpClient = config.getHttpClient();
5051
this.debugTruncateBytes = config.getDebugTruncateBytes();
5152
this.accountId = config.getAccountId();
52-
this.retryStrategyPicker = new RequestBasedRetryStrategyPicker(config.getHost());
5353
this.isDebugHeaders = config.isDebugHeaders();
54+
55+
if (config.getDisableRetries()) {
56+
this.retryStrategyPicker = new NoRetryStrategyPicker();
57+
} else {
58+
this.retryStrategyPicker = new RequestBasedRetryStrategyPicker(config.getHost());
59+
}
60+
5461
return this;
5562
}
5663

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,10 @@ public class DatabricksConfig {
164164
@ConfigAttribute(env = "DATABRICKS_DISABLE_ASYNC_TOKEN_REFRESH")
165165
private Boolean disableAsyncTokenRefresh;
166166

167+
/** Disable retries by default when set to true. */
168+
@ConfigAttribute(env = "DATABRICKS_DISABLE_RETRIES")
169+
private Boolean disableRetries;
170+
167171
/**
168172
* The duration to wait for a browser response during U2M authentication before timing out. If set
169173
* to 0 or null, the connector waits for an indefinite amount of time.
@@ -609,6 +613,15 @@ public DatabricksConfig setDisableAsyncTokenRefresh(boolean disableAsyncTokenRef
609613
return this;
610614
}
611615

616+
public boolean getDisableRetries() {
617+
return disableRetries != null && disableRetries;
618+
}
619+
620+
public DatabricksConfig setDisableRetries(boolean disableRetries) {
621+
this.disableRetries = disableRetries;
622+
return this;
623+
}
624+
612625
public Duration getOAuthBrowserAuthTimeout() {
613626
return oauthBrowserAuthTimeout;
614627
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package com.databricks.sdk.core.retry;
2+
3+
import com.databricks.sdk.core.DatabricksError;
4+
5+
/**
6+
* This class implements a retry strategy that never retries any requests. All requests are
7+
* considered non-retriable regardless of the error type or status code.
8+
*/
9+
public class NoRetryStrategy implements RetryStrategy {
10+
11+
@Override
12+
public boolean isRetriable(DatabricksError databricksError) {
13+
return false;
14+
}
15+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package com.databricks.sdk.core.retry;
2+
3+
import com.databricks.sdk.core.http.Request;
4+
5+
/**
6+
* A RetryStrategyPicker that always returns a NoRetryStrategy, effectively disabling retries for
7+
* all requests regardless of their type or characteristics.
8+
*/
9+
public class NoRetryStrategyPicker implements RetryStrategyPicker {
10+
private static final NoRetryStrategy NO_RETRY_STRATEGY = new NoRetryStrategy();
11+
12+
/**
13+
* Returns a NoRetryStrategy for any given request, effectively disabling retries.
14+
*
15+
* @param request The request for which the retry strategy is needed (ignored).
16+
* @return A NoRetryStrategy instance that will never retry any request.
17+
*/
18+
@Override
19+
public RetryStrategy getRetryStrategy(Request request) {
20+
return NO_RETRY_STRATEGY;
21+
}
22+
}

databricks-sdk-java/src/test/java/com/databricks/sdk/core/ApiClientTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,40 @@ public HeaderFactory configure(DatabricksConfig config) {
410410
}
411411
}
412412

413+
@Test
414+
void verifyNoRetryWhenRetriesDisabled() throws IOException {
415+
Request req = getBasicRequest();
416+
DatabricksConfig config =
417+
new DatabricksConfig()
418+
.setHost("http://my.host")
419+
.setDisableRetries(true)
420+
.setCredentialsProvider(new DummyCredentialsProvider());
421+
ApiClient client =
422+
getApiClient(
423+
config, req, Arrays.asList(getTooManyRequestsResponse(req), getSuccessResponse(req)));
424+
425+
DatabricksError exception =
426+
assertThrows(
427+
DatabricksError.class,
428+
() ->
429+
client.execute(
430+
new Request("GET", req.getUri().getPath()), MyEndpointResponse.class));
431+
432+
assertEquals("TOO_MANY_REQUESTS", exception.getErrorCode());
433+
assertEquals(429, exception.getStatusCode());
434+
}
435+
436+
@Test
437+
void verifyRetriesWorkWhenEnabled() throws IOException {
438+
Request req = getBasicRequest();
439+
// Verify that the client retries by default.
440+
runApiClientTest(
441+
req,
442+
Arrays.asList(getTooManyRequestsResponse(req), getSuccessResponse(req)),
443+
MyEndpointResponse.class,
444+
new MyEndpointResponse().setKey("value")); // should succeed after retry
445+
}
446+
413447
@Test
414448
void populateHostFromCredentialProvider() throws IOException {
415449
Request req = getBasicRequest();

databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,21 @@ public void testEnvironmentVariableLoading() {
282282
assertEquals(Integer.valueOf(100), config.getDebugTruncateBytes());
283283
assertEquals(Integer.valueOf(50), config.getRateLimit());
284284
}
285+
286+
@Test
287+
public void testDisableRetriesSetAndGet() {
288+
DatabricksConfig config = new DatabricksConfig().setDisableRetries(true);
289+
assertEquals(true, config.getDisableRetries());
290+
}
291+
292+
@Test
293+
public void testDisableRetriesEnvironmentVariable() {
294+
Map<String, String> env = new HashMap<>();
295+
env.put("DATABRICKS_DISABLE_RETRIES", "true");
296+
297+
DatabricksConfig config = new DatabricksConfig();
298+
config.resolve(new Environment(env, new ArrayList<>(), System.getProperty("os.name")));
299+
300+
assertEquals(true, config.getDisableRetries());
301+
}
285302
}

0 commit comments

Comments
 (0)