From 150235f46d033fe1e5de29f12d7803bee1174026 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 14 Oct 2026 09:39:59 +0000 Subject: [PATCH 1/4] Retry workspace get-status on transient failures during sync EnsureRemotePathIsUsable validates the sync target with workspace/get-status. A transient server error or a dropped connection (e.g. REFUSED_STREAM, #6472) on that read aborted the whole deploy. Route the calls through libs/retry with a new IsTransientRead predicate: on top of the 5xx/408 that IsTransient covers, an idempotent read is also safe to replay after a transport-level failure (*url.Error). Co-authored-by: Isaac --- .nextchanges/bundles/get-status-retries.md | 1 + acceptance/cmd/sync/get-status-504/hello.py | 1 + .../cmd/sync/get-status-504/out.test.toml | 1 + acceptance/cmd/sync/get-status-504/output.txt | 4 ++++ acceptance/cmd/sync/get-status-504/script | 22 +++++++++++++++++++ acceptance/cmd/sync/get-status-504/test.toml | 1 + acceptance/cmd/sync/output.txt | 6 ++++- libs/retry/retry.go | 15 +++++++++++++ libs/retry/retry_test.go | 16 ++++++++++++++ libs/sync/path.go | 16 +++++++++++--- 10 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 .nextchanges/bundles/get-status-retries.md create mode 100644 acceptance/cmd/sync/get-status-504/hello.py create mode 100644 acceptance/cmd/sync/get-status-504/out.test.toml create mode 100644 acceptance/cmd/sync/get-status-504/output.txt create mode 100644 acceptance/cmd/sync/get-status-504/script create mode 100644 acceptance/cmd/sync/get-status-504/test.toml diff --git a/.nextchanges/bundles/get-status-retries.md b/.nextchanges/bundles/get-status-retries.md new file mode 100644 index 00000000000..cea8906ffed --- /dev/null +++ b/.nextchanges/bundles/get-status-retries.md @@ -0,0 +1 @@ +* `databricks bundle deploy`, `databricks bundle sync` and `databricks sync` now retry the workspace path check (`workspace/get-status`) when it fails with a transient server error (5xx or 408) or a dropped connection such as `REFUSED_STREAM`, instead of failing. diff --git a/acceptance/cmd/sync/get-status-504/hello.py b/acceptance/cmd/sync/get-status-504/hello.py new file mode 100644 index 00000000000..11b15b1a458 --- /dev/null +++ b/acceptance/cmd/sync/get-status-504/hello.py @@ -0,0 +1 @@ +print("hello") diff --git a/acceptance/cmd/sync/get-status-504/out.test.toml b/acceptance/cmd/sync/get-status-504/out.test.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/cmd/sync/get-status-504/out.test.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/cmd/sync/get-status-504/output.txt b/acceptance/cmd/sync/get-status-504/output.txt new file mode 100644 index 00000000000..863d79b61d2 --- /dev/null +++ b/acceptance/cmd/sync/get-status-504/output.txt @@ -0,0 +1,4 @@ +Warn: retrying after 504 Gateway Timeout from GET /api/2.0/workspace/get-status +Action: PUT: hello.py +Uploaded hello.py +Initial Sync Complete diff --git a/acceptance/cmd/sync/get-status-504/script b/acceptance/cmd/sync/get-status-504/script new file mode 100644 index 00000000000..2b101d707c7 --- /dev/null +++ b/acceptance/cmd/sync/get-status-504/script @@ -0,0 +1,22 @@ +# .gitignore excludes itself and .databricks/ so that hello.py is the only synced file. +mkdir .git +cat > .gitignore << EOF2 +.databricks/ +.gitignore +EOF2 + +# Use an ephemeral remote directory so concurrent test runs do not interfere. +remote_dir="/Users/$CURRENT_USER_NAME/sync-getstatus-504-$UNIQUE_NAME" + +cleanup() { + MSYS_NO_PATHCONV=1 $CLI workspace delete --recursive "$remote_dir" 2>/dev/null || true + rm -rf .git .gitignore .databricks +} +trap cleanup EXIT + +# Inject a single 504 on the first workspace get-status, the call that validates the sync +# target (issue #6472). The read has no side effects, so it is retried and the sync succeeds. +fault.py "GET /api/2.0/workspace/get-status" 504 0 1 + +# MSYS_NO_PATHCONV=1 prevents Git Bash on Windows from converting /Users/... to C:/Program Files/Git/Users/... +MSYS_NO_PATHCONV=1 $CLI sync . "$remote_dir" diff --git a/acceptance/cmd/sync/get-status-504/test.toml b/acceptance/cmd/sync/get-status-504/test.toml new file mode 100644 index 00000000000..2889b56fb03 --- /dev/null +++ b/acceptance/cmd/sync/get-status-504/test.toml @@ -0,0 +1 @@ +Env.DATABRICKS_BUNDLE_RETRY_INTERVAL_MS = "100" diff --git a/acceptance/cmd/sync/output.txt b/acceptance/cmd/sync/output.txt index 4695359a9d9..2978ee5f328 100644 --- a/acceptance/cmd/sync/output.txt +++ b/acceptance/cmd/sync/output.txt @@ -1,6 +1,6 @@ >>> [CLI] sync . /Users/[USERNAME] -Action: PUT: .gitignore, dryrun-missing-remote/hello.py, dryrun-missing-remote/out.test.toml, dryrun-missing-remote/output.txt, dryrun/out.test.toml, dryrun/output.txt, project-folder/app.py, project-folder/app.yaml, project-folder/query.sql, upload-504/hello.py, upload-504/out.test.toml, upload-504/output.txt +Action: PUT: .gitignore, dryrun-missing-remote/hello.py, dryrun-missing-remote/out.test.toml, dryrun-missing-remote/output.txt, dryrun/out.test.toml, dryrun/output.txt, get-status-504/hello.py, get-status-504/out.test.toml, get-status-504/output.txt, project-folder/app.py, project-folder/app.yaml, project-folder/query.sql, upload-504/hello.py, upload-504/out.test.toml, upload-504/output.txt Initial Sync Complete Uploaded .gitignore Uploaded dryrun @@ -10,6 +10,10 @@ Uploaded dryrun-missing-remote/out.test.toml Uploaded dryrun-missing-remote/output.txt Uploaded dryrun/out.test.toml Uploaded dryrun/output.txt +Uploaded get-status-504 +Uploaded get-status-504/hello.py +Uploaded get-status-504/out.test.toml +Uploaded get-status-504/output.txt Uploaded project-folder Uploaded project-folder/app.py Uploaded project-folder/app.yaml diff --git a/libs/retry/retry.go b/libs/retry/retry.go index 81b894cbb88..5c9701bd1fc 100644 --- a/libs/retry/retry.go +++ b/libs/retry/retry.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "strconv" "time" @@ -50,6 +51,20 @@ func IsTransient(ctx context.Context, err error) bool { return apiErr.StatusCode >= 500 || apiErr.StatusCode == http.StatusRequestTimeout } +// IsTransientRead returns true for errors worth retrying on an idempotent read. +// Besides the transient server errors IsTransient covers, a read can also be retried after +// a transport-level failure that produced no HTTP response, such as the REFUSED_STREAM in +// https://github.com/databricks/cli/issues/6472: replaying a read has no side effects, so +// completing the round trip (on a fresh connection) is safe. These failures surface as a +// *url.Error, whereas an error carrying an HTTP status is an *apierr.APIError. +func IsTransientRead(ctx context.Context, err error) bool { + if IsTransient(ctx, err) { + return true + } + _, ok := errors.AsType[*url.Error](err) + return ok +} + // With retries fn while check returns true for the error, up to MaxRetries times. func With[T any](ctx context.Context, check func(context.Context, error) bool, fn func() (T, error)) (T, error) { wait := interval(ctx) diff --git a/libs/retry/retry_test.go b/libs/retry/retry_test.go index 231389d4aa2..0f22bde4bd5 100644 --- a/libs/retry/retry_test.go +++ b/libs/retry/retry_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "testing" "time" @@ -26,6 +27,21 @@ func TestIsTransient(t *testing.T) { assert.False(t, IsTransient(ctx, errors.New("boom"))) } +func TestIsTransientRead(t *testing.T) { + ctx := t.Context() + // Everything IsTransient covers is transient for a read too. + assert.True(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusInternalServerError})) + assert.True(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusRequestTimeout})) + // A transport-level failure with no HTTP response is safe to replay on a read (issue #6472). + refusedStream := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: errors.New("stream error: stream ID 1; REFUSED_STREAM; received from peer")} + assert.True(t, IsTransientRead(ctx, refusedStream)) + // Not transient: retried by the SDK, client errors, and plain non-url errors. + assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusServiceUnavailable})) + assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusNotFound})) + assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusForbidden})) + assert.False(t, IsTransientRead(ctx, errors.New("boom"))) +} + // testContext shortens the pause between attempts. func testContext(t *testing.T) context.Context { return env.Set(t.Context(), IntervalMsVariable, "1") diff --git a/libs/sync/path.go b/libs/sync/path.go index 6b60ef682de..ff177800eef 100644 --- a/libs/sync/path.go +++ b/libs/sync/path.go @@ -7,12 +7,22 @@ import ( "strings" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/retry" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/databricks-sdk-go/service/workspace" ) +// getStatusWithRetry returns the workspace object at remotePath, retrying transient failures. +// The read has no side effects, so a refused HTTP/2 stream or other transient error +// (https://github.com/databricks/cli/issues/6472) is safe to replay instead of failing the sync. +func getStatusWithRetry(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string) (*workspace.ObjectInfo, error) { + return retry.With(ctx, retry.IsTransientRead, func() (*workspace.ObjectInfo, error) { + return wsc.Workspace.GetStatusByPath(ctx, remotePath) + }) +} + func repoPathForPath(me *iam.User, remotePath string) string { base := path.Clean("/Repos/" + me.UserName) remotePath = path.Clean(remotePath) @@ -40,7 +50,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie // Ensure that the remote path exists. // If it is a repo, it has to exist. // If it is a workspace path, it may not exist. - info, err := wsc.Workspace.GetStatusByPath(ctx, remotePath) + info, err := getStatusWithRetry(ctx, wsc, remotePath) if err != nil { // We only deal with 404s below. if !apierr.IsMissing(err) { @@ -50,7 +60,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie // If the path is nested under a repo, the repo has to exist. if strings.HasPrefix(remotePath, "/Repos/") { repoPath := repoPathForPath(me, remotePath) - _, err = wsc.Workspace.GetStatusByPath(ctx, repoPath) + _, err = getStatusWithRetry(ctx, wsc, repoPath) if err != nil && apierr.IsMissing(err) { return fmt.Errorf("%s does not exist; please create it first", repoPath) } @@ -66,7 +76,7 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie if err != nil { return fmt.Errorf("unable to create directory at %s: %w", remotePath, err) } - info, err = wsc.Workspace.GetStatusByPath(ctx, remotePath) + info, err = getStatusWithRetry(ctx, wsc, remotePath) if err != nil { return err } From d339309c39cd9f4dbf5cc345833f9f8d5a38e741 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 14 Oct 2026 09:43:09 +0000 Subject: [PATCH 2/4] Add PR link to changelog fragment Co-authored-by: Isaac --- .nextchanges/bundles/get-status-retries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/bundles/get-status-retries.md b/.nextchanges/bundles/get-status-retries.md index cea8906ffed..b5347007b84 100644 --- a/.nextchanges/bundles/get-status-retries.md +++ b/.nextchanges/bundles/get-status-retries.md @@ -1 +1 @@ -* `databricks bundle deploy`, `databricks bundle sync` and `databricks sync` now retry the workspace path check (`workspace/get-status`) when it fails with a transient server error (5xx or 408) or a dropped connection such as `REFUSED_STREAM`, instead of failing. +* `databricks bundle deploy`, `databricks bundle sync` and `databricks sync` now retry the workspace path check (`workspace/get-status`) when it fails with a transient server error (5xx or 408) or a dropped connection such as `REFUSED_STREAM`, instead of failing. ([#7044](https://github.com/databricks/cli/pull/7044)) From a17e854b433c3dcc1e030ad1cbe67ff9a5220ea4 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 14 Oct 2026 11:19:17 +0000 Subject: [PATCH 3/4] retry: scope get-status transport retry to REFUSED_STREAM Matching every *url.Error restarted the SDK's RetryTimeout loop (5m default) for the transport errors it already retries (connection reset/refused, i/o timeout, TLS), turning a persistent failure's ~5m into ~15m. Retry only the HTTP/2 REFUSED_STREAM the SDK leaves unhandled (#6472). Co-authored-by: Isaac --- .nextchanges/bundles/get-status-retries.md | 2 +- libs/retry/retry.go | 16 +++++++++------- libs/retry/retry_test.go | 5 ++++- libs/sync/path.go | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.nextchanges/bundles/get-status-retries.md b/.nextchanges/bundles/get-status-retries.md index b5347007b84..7213a49fe7b 100644 --- a/.nextchanges/bundles/get-status-retries.md +++ b/.nextchanges/bundles/get-status-retries.md @@ -1 +1 @@ -* `databricks bundle deploy`, `databricks bundle sync` and `databricks sync` now retry the workspace path check (`workspace/get-status`) when it fails with a transient server error (5xx or 408) or a dropped connection such as `REFUSED_STREAM`, instead of failing. ([#7044](https://github.com/databricks/cli/pull/7044)) +* `databricks bundle deploy`, `databricks bundle sync` and `databricks sync` now retry the workspace path check (`workspace/get-status`) when it fails with a transient server error (5xx or 408) or an HTTP/2 `REFUSED_STREAM`, instead of failing. ([#7044](https://github.com/databricks/cli/pull/7044)) diff --git a/libs/retry/retry.go b/libs/retry/retry.go index 5c9701bd1fc..97ac0f0d6d6 100644 --- a/libs/retry/retry.go +++ b/libs/retry/retry.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "time" "github.com/databricks/cli/libs/env" @@ -51,18 +52,19 @@ func IsTransient(ctx context.Context, err error) bool { return apiErr.StatusCode >= 500 || apiErr.StatusCode == http.StatusRequestTimeout } -// IsTransientRead returns true for errors worth retrying on an idempotent read. -// Besides the transient server errors IsTransient covers, a read can also be retried after -// a transport-level failure that produced no HTTP response, such as the REFUSED_STREAM in -// https://github.com/databricks/cli/issues/6472: replaying a read has no side effects, so -// completing the round trip (on a fresh connection) is safe. These failures surface as a -// *url.Error, whereas an error carrying an HTTP status is an *apierr.APIError. +// IsTransientRead returns true for errors worth retrying on an idempotent read: the transient +// server errors IsTransient covers, plus the HTTP/2 REFUSED_STREAM in issue #6472, where the +// server refused the stream before processing the request so the read is safe to replay. The +// SDK already retries the transport errors it recognizes (connection reset/refused, i/o timeout, +// TLS handshake) for its full RetryTimeout, so retrying every *url.Error would restart that +// multi-minute loop; REFUSED_STREAM is the gap it leaves. net/http bundles unexported http2 +// types, so we match the message, as the SDK does for its own transient url errors. func IsTransientRead(ctx context.Context, err error) bool { if IsTransient(ctx, err) { return true } _, ok := errors.AsType[*url.Error](err) - return ok + return ok && strings.Contains(err.Error(), "REFUSED_STREAM") } // With retries fn while check returns true for the error, up to MaxRetries times. diff --git a/libs/retry/retry_test.go b/libs/retry/retry_test.go index 0f22bde4bd5..1a54904d40c 100644 --- a/libs/retry/retry_test.go +++ b/libs/retry/retry_test.go @@ -32,9 +32,12 @@ func TestIsTransientRead(t *testing.T) { // Everything IsTransient covers is transient for a read too. assert.True(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusInternalServerError})) assert.True(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusRequestTimeout})) - // A transport-level failure with no HTTP response is safe to replay on a read (issue #6472). + // REFUSED_STREAM is the transport failure the SDK leaves unretried, so we replay the read (issue #6472). refusedStream := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: errors.New("stream error: stream ID 1; REFUSED_STREAM; received from peer")} assert.True(t, IsTransientRead(ctx, refusedStream)) + // Transport errors the SDK already retries for its full RetryTimeout must not be retried again. + connRefused := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: errors.New("dial tcp 10.0.0.1:443: connect: connection refused")} + assert.False(t, IsTransientRead(ctx, connRefused)) // Not transient: retried by the SDK, client errors, and plain non-url errors. assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusServiceUnavailable})) assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusNotFound})) diff --git a/libs/sync/path.go b/libs/sync/path.go index ff177800eef..d85699cf55c 100644 --- a/libs/sync/path.go +++ b/libs/sync/path.go @@ -15,7 +15,7 @@ import ( ) // getStatusWithRetry returns the workspace object at remotePath, retrying transient failures. -// The read has no side effects, so a refused HTTP/2 stream or other transient error +// The read has no side effects, so a transient server error or a refused HTTP/2 stream // (https://github.com/databricks/cli/issues/6472) is safe to replay instead of failing the sync. func getStatusWithRetry(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string) (*workspace.ObjectInfo, error) { return retry.With(ctx, retry.IsTransientRead, func() (*workspace.ObjectInfo, error) { From 6684fff52c5e08ec3a896599069b6e6e45cc490f Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 14 Oct 2026 12:48:57 +0000 Subject: [PATCH 4/4] retry: assert a cancelled get-status read is not retried Co-authored-by: Isaac --- libs/retry/retry_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/retry/retry_test.go b/libs/retry/retry_test.go index 1a54904d40c..0a9006cbedc 100644 --- a/libs/retry/retry_test.go +++ b/libs/retry/retry_test.go @@ -38,6 +38,9 @@ func TestIsTransientRead(t *testing.T) { // Transport errors the SDK already retries for its full RetryTimeout must not be retried again. connRefused := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: errors.New("dial tcp 10.0.0.1:443: connect: connection refused")} assert.False(t, IsTransientRead(ctx, connRefused)) + // A cancelled request is a *url.Error too, but not REFUSED_STREAM, so it fails fast rather than logging a retry. + cancelled := &url.Error{Op: "Get", URL: "https://example.test/api/2.0/workspace/get-status", Err: context.Canceled} + assert.False(t, IsTransientRead(ctx, cancelled)) // Not transient: retried by the SDK, client errors, and plain non-url errors. assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusServiceUnavailable})) assert.False(t, IsTransientRead(ctx, &apierr.APIError{StatusCode: http.StatusNotFound}))