direct: retry recreate's create while the previous delete clears

.nextchanges/bundles/postgres-synced-table-recreate.md

@@ -1 +1 @@
-* Fix recreating a postgres synced table sometimes failing with a 409 ALREADY_EXISTS error while the previous table is still being deleted. ([#6728](https://github.com/databricks/cli/pull/6728))
+* Fix recreating a postgres synced table sometimes failing with a 409 ALREADY_EXISTS error while the previous table is still being deleted. ([#6728](https://github.com/databricks/cli/pull/6728), [#6748](https://github.com/databricks/cli/pull/6748))

acceptance/bundle/resources/apps/recreate_rename_conflict/app_a/app.py

@@ -0,0 +1 @@
+print("Hello world!")

acceptance/bundle/resources/apps/recreate_rename_conflict/app_b/app.py

@@ -0,0 +1 @@
+print("Hello world!")

acceptance/bundle/resources/apps/recreate_rename_conflict/databricks.yml

@@ -0,0 +1,11 @@
+bundle:
+  name: test-bundle
+
+resources:
+  apps:
+    app_a:
+      name: app-alpha
+      source_code_path: ./app_a
+    app_b:
+      name: app-beta
+      source_code_path: ./app_b

acceptance/bundle/resources/apps/recreate_rename_conflict/output.txt

@@ -0,0 +1,46 @@
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Created apps.app_a
+Created apps.app_b
+Files: 5 uploaded, 0 deleted
+Resources: 2 created, 0 changed, 0 deleted, 0 unchanged
+
+=== Renaming app_a onto app_b's name fails fast (create attempted once, no retry)
+>>> update_file.py databricks.yml name: app-alpha name: app-beta
+
+>>> musterr [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Error: cannot recreate resources.apps.app_a: An app with the same name already exists: app-beta (409 RESOURCE_ALREADY_EXISTS)
+
+Endpoint: POST [DATABRICKS_URL]/api/2.0/apps?no_compute=true
+HTTP Status: 409 Conflict
+API error_code: RESOURCE_ALREADY_EXISTS
+API message: An app with the same name already exists: app-beta
+
+Files: 1 uploaded, 0 deleted
+
+>>> print_requests.py //apps
+{
+  "method": "DELETE",
+  "path": "/api/2.0/apps/app-alpha"
+}
+{
+  "method": "POST",
+  "path": "/api/2.0/apps",
+  "q": {
+    "no_compute": "true"
+  },
+  "body": {
+    "description": "",
+    "name": "app-beta"
+  }
+}
+
+>>> [CLI] bundle destroy --auto-approve
+The following resources will be deleted:
+  delete resources.apps.app_b
+
+All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
+
+Destroy: 1 deleted

acceptance/bundle/resources/apps/recreate_rename_conflict/script

@@ -0,0 +1,13 @@
+trace $CLI bundle deploy
+
+title "Renaming app_a onto app_b's name fails fast (create attempted once, no retry)"
+# name is a provided-id field, so the plan recreates app_a: delete app-alpha, then
+# create app-beta — which app_b already owns. The create must surface the conflict
+# immediately. Reset the recorded requests first so print_requests below proves the
+# recreate attempts the create exactly once, rather than retrying it in a burst.
+trace update_file.py databricks.yml "name: app-alpha" "name: app-beta"
+rm -f "$OUT_REQUESTS"
+trace musterr $CLI bundle deploy
+trace print_requests.py //apps
+
+trace $CLI bundle destroy --auto-approve

acceptance/bundle/resources/apps/recreate_rename_conflict/test.toml

@@ -0,0 +1,8 @@
+Cloud = false
+
+# Record requests so the golden proves the rename's create is attempted once (the
+# guard fails fast) rather than retried in a burst.
+RecordRequests = true
+
+# The same-id guard that makes a rename-to-existing fail fast is a direct-engine feature.
+EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]

acceptance/bundle/resources/postgres_synced_tables/recreate_capped/databricks.yml.tmpl → acceptance/bundle/resources/postgres_synced_tables/recreate_conflict_retry/databricks.yml

@@ -1,39 +1,39 @@
 bundle:
-  name: recreate-capped-postgres-synced-table-$UNIQUE_NAME
+  name: test-bundle
 
 sync:
   paths: []
 
 resources:
   schemas:
     pipeline_storage:
-      name: pipeline_storage_$UNIQUE_NAME
+      name: pipeline_storage
       catalog_name: main
       comment: "Pipeline storage for the synced-table recreate test"
 
   postgres_projects:
     my_project:
-      project_id: test-pg-proj-$UNIQUE_NAME
+      project_id: test-pg-proj
       display_name: "Test Project for Synced Table Recreate"
       pg_version: 17
 
   postgres_catalogs:
     my_catalog:
-      catalog_id: lakebase_test_$UNIQUE_NAME
+      catalog_id: lakebase_test
       branch: ${resources.postgres_projects.my_project.id}/branches/production
       postgres_database: appdb
       create_database_if_missing: true
 
   postgres_synced_tables:
     my_table:
       synced_table_id: ${resources.postgres_catalogs.my_catalog.catalog_id}.public.trips_synced
-      source_table_full_name: main.source_$UNIQUE_NAME.trips_source
+      source_table_full_name: main.source.trips_source
       primary_key_columns: ["tpep_pickup_datetime"]
       scheduling_policy: SNAPSHOT
       postgres_database: appdb
       branch: ${resources.postgres_projects.my_project.id}/branches/production
       create_database_objects_if_missing: true
-      timeseries_key: $TIMESERIES_KEY
+      timeseries_key: tpep_pickup_datetime
       new_pipeline_spec:
         storage_catalog: ${resources.schemas.pipeline_storage.catalog_name}
         storage_schema: ${resources.schemas.pipeline_storage.name}

acceptance/bundle/resources/postgres_synced_tables/recreate_capped/output.txt → acceptance/bundle/resources/postgres_synced_tables/recreate_conflict_retry/output.txt

@@ -1,26 +1,28 @@
 
 >>> [CLI] bundle deploy
-Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/recreate-capped-postgres-synced-table-[UNIQUE_NAME]/default/files...
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
 Created postgres_catalogs.my_catalog
 Created postgres_projects.my_project
 Created postgres_synced_tables.my_table
 Created schemas.pipeline_storage
 Files: 0 uploaded, 0 deleted
 Resources: 4 created, 0 changed, 0 deleted, 0 unchanged
 
-=== The delete never completes; the capped wait times out and the recreate fails
->>> DATABRICKS_BUNDLE_RESOURCE_MAX_WAIT=1 errcode [CLI] bundle deploy
-Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/recreate-capped-postgres-synced-table-[UNIQUE_NAME]/default/files...
-Error: cannot recreate resources.postgres_synced_tables.my_table: waiting after deleting id=synced_tables/lakebase_test_[UNIQUE_NAME].public.trips_synced: timed out: synced table still exists, waiting for deletion to complete
+=== The create races the still-finishing delete (409 ALREADY_EXISTS); recreate retries and succeeds
+>>> update_file.py databricks.yml timeseries_key: tpep_pickup_datetime timeseries_key: tpep_dropoff_datetime
 
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Warn: deploying resources.postgres_synced_tables.my_table: Create still conflicts; the previous delete is likely still finishing, retrying: Failing setup of Delta sync table: Destination table lakebase_test.public.trips_synced already exists
+Recreated postgres_synced_tables.my_table
 Files: 0 uploaded, 0 deleted
-
-Exit code: 1
+Resources: 1 created, 0 changed, 1 deleted, 3 unchanged
 
 >>> [CLI] bundle destroy --auto-approve
 The following resources will be deleted:
   delete resources.postgres_catalogs.my_catalog
   delete resources.postgres_projects.my_project
+  delete resources.postgres_synced_tables.my_table
   delete resources.schemas.pipeline_storage
 
 This action will result in the deletion of the following UC schemas. Any underlying data may be lost:
@@ -30,6 +32,6 @@ This action will result in the deletion of the following Lakebase projects along
 all their branches, databases, and endpoints. All data stored in them will be permanently lost:
   delete resources.postgres_projects.my_project
 
-All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/recreate-capped-postgres-synced-table-[UNIQUE_NAME]/default
+All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
 
-Destroy: 3 deleted
+Destroy: 4 deleted

acceptance/bundle/resources/postgres_synced_tables/recreate_conflict_retry/script

@@ -0,0 +1,13 @@
+cleanup() {
+    trace $CLI bundle destroy --auto-approve
+}
+trap cleanup EXIT
+
+trace $CLI bundle deploy
+
+title "The create races the still-finishing delete (409 ALREADY_EXISTS); recreate retries and succeeds"
+# Toggle a recreate-on-change field (timeseries_key) to force a recreate. The synced-table
+# record is gone from GET so the delete-wait completes, but the first create conflicts
+# because the destination table is still being dropped; the recreate retries and succeeds.
+trace update_file.py databricks.yml "timeseries_key: tpep_pickup_datetime" "timeseries_key: tpep_dropoff_datetime"
+trace $CLI bundle deploy

acceptance/bundle/resources/postgres_synced_tables/recreate_conflict_retry/test.toml

@@ -0,0 +1,9 @@
+# Local-only, direct-only: exercises the recreate create-retry when a synced table's
+# delete is asynchronous. INJECT_STALE_ON_DIRECT makes the testserver drop the synced-table
+# record immediately (so the delete-wait completes) but reject the next create for the same
+# id once with 409 ALREADY_EXISTS, modelling the destination Postgres table still being
+# dropped. The recreate retries the create and succeeds. Not run on cloud (needs a real
+# source table) or on terraform (direct-only flag).
+Cloud = false
+EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
+Env.INJECT_STALE_ON_DIRECT = "1"

bundle/direct/apply.go

@@ -7,12 +7,15 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
+	"time"
 
 	"github.com/databricks/cli/bundle/deployplan"
 	"github.com/databricks/cli/bundle/direct/dresources"
 	"github.com/databricks/cli/bundle/direct/dstate"
 	"github.com/databricks/cli/libs/log"
+	"github.com/databricks/cli/libs/structs/structpath"
 	"github.com/databricks/databricks-sdk-go/apierr"
+	"github.com/databricks/databricks-sdk-go/retries"
 )
 
 func (d *DeploymentUnit) withResourceKey(ctx context.Context) context.Context {
@@ -46,7 +49,7 @@ func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState,
 
 	switch actionType {
 	case deployplan.Recreate:
-		return d.Recreate(ctx, db, oldID, newState)
+		return d.Recreate(ctx, db, oldID, newState, planEntry)
 	case deployplan.Update:
 		return d.Update(ctx, db, oldID, newState, planEntry)
 	case deployplan.UpdateWithID:
@@ -106,7 +109,7 @@ func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState,
 	return nil
 }
 
-func (d *DeploymentUnit) Recreate(ctx context.Context, db *dstate.DeploymentState, oldID string, newState any) error {
+func (d *DeploymentUnit) Recreate(ctx context.Context, db *dstate.DeploymentState, oldID string, newState any, planEntry *deployplan.PlanEntry) error {
 	oldState, err := d.loadPersistedState(db)
 	if err != nil {
 		return err
@@ -146,9 +149,79 @@ func (d *DeploymentUnit) Recreate(ctx context.Context, db *dstate.DeploymentStat
 		return fmt.Errorf("waiting after deleting id=%s: %w", oldID, err)
 	}
 
-	return d.Create(ctx, db, newState)
+	// The delete-wait above only observes the resource's own read (e.g. the synced-table
+	// record). A delete can leave *backing* objects behind that it can't see — a synced
+	// table's destination Postgres table is dropped on a slower schedule — so the create
+	// of the same id can still conflict. Retry the create until that teardown finishes,
+	// then surface the result. We match the whole conflict class (ErrResourceConflict)
+	// rather than a single error code: the same lingering-teardown race can surface as
+	// ALREADY_EXISTS or RESOURCE_ALREADY_EXISTS depending on the resource, and both mean
+	// the same thing here.
+	//
+	// Only when the recreate re-creates the *same* id, though. If it changed a
+	// provided-id field (e.g. renamed an app, or pointed a synced table at a new
+	// synced_table_id), the create targets a *different* id that a separate,
+	// pre-existing resource already owns — waiting cannot free that name, so surface
+	// the conflict immediately instead of retrying for minutes. Any non-conflict error
+	// is returned immediately too.
+	idChanged := d.recreateChangedID(planEntry)
+	var createErr error
+	_, _ = retries.Poll[struct{}](ctx, recreateConflictRetryTimeout, func() (*struct{}, *retries.Err) {
+		createErr = d.Create(ctx, db, newState)
+		switch {
+		case createErr == nil:
+			return &struct{}{}, nil
+		case !idChanged && errors.Is(createErr, apierr.ErrResourceConflict):
+			log.Warnf(ctx, "Create still conflicts; the previous delete is likely still finishing, retrying: %s", createErr)
+			return nil, retries.Continues("create still conflicts with the deleting resource")
+		default:
+			return nil, retries.Halt(createErr)
+		}
+	})
+	return createErr
 }
 
+// recreateChangedID reports whether this recreate edits a field that composes the
+// resource's id (a provided_id_field, e.g. an app's name or a synced table's
+// synced_table_id). When it does, the create targets a different id than the one
+// just deleted, so an ALREADY_EXISTS can only mean a separate, pre-existing resource
+// owns that id — waiting never frees it. When it does not, the recreate re-creates
+// the same id, so ALREADY_EXISTS can only be the just-deleted resource still tearing
+// down.
+func (d *DeploymentUnit) recreateChangedID(planEntry *deployplan.PlanEntry) bool {
+	if planEntry == nil {
+		return false
+	}
+	for field, ch := range planEntry.Changes {
+		if ch.Action == deployplan.Skip {
+			continue
+		}
+		path, err := structpath.ParsePath(field)
+		if err != nil {
+			continue
+		}
+		if changesProvidedID(d.Adapter.ResourceConfig(), path) || changesProvidedID(d.Adapter.GeneratedResourceConfig(), path) {
+			return true
+		}
+	}
+	return false
+}
+
+func changesProvidedID(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode) bool {
+	if cfg == nil {
+		return false
+	}
+	_, ok := findMatchingRule(path, cfg.ProvidedIDFields)
+	return ok
+}
+
+// recreateConflictRetryTimeout caps how long recreate retries a create that keeps
+// failing with ALREADY_EXISTS because the just-deleted resource (or its backing
+// objects) is still being torn down. No measured teardown latency to derive this
+// from, so it matches deleteIndexTimeout (15m); if it's exceeded the recreate fails
+// and the next deploy re-creates (state was already dropped).
+const recreateConflictRetryTimeout = 15 * time.Minute
+
 func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState, id string, newState any, planEntry *deployplan.PlanEntry) error {
 	if !d.Adapter.HasDoUpdate() {
 		return fmt.Errorf("internal error: DoUpdate not implemented for resource %s", d.ResourceKey)