From c3a7d17d1d97db17a585cbb06abfa3ff6a58d4e0 Mon Sep 17 00:00:00 2001 From: "Lennart Kats (databricks)" Date: Wed, 17 Apr 2024 18:59:39 -0700 Subject: [PATCH] Disable locking for development mode (#1302) ## Changes This changes `databricks bundle deploy` so that it skips the lock acquisition/release step for a `mode: development` target: * This saves about 2 seconds (measured over 100 runs on a quiet/busy workspace). * This helps avoid the `deploy lock acquired by lennart@company.com at 2024-02-28 15:48:38.40603 +0100 CET. Use --force-lock to override` error * Risk: this may cause deployment conflicts, but since dev mode deployments are always scoped to a user, that risk should be minimal Update after discussion: * This behavior can now be disabled via a setting. * Docs PR: https://github.com/databricks/docs/pull/15873 ## Measurements ### 100 deployments of the "python_default" project to an empty workspace _Before this branch:_ p50 time: 11.479 seconds p90 time: 11.757 seconds _After this branch:_ p50 time: 9.386 seconds p90 time: 9.599 seconds ### 100 deployments of the "python_default" project to a busy (staging) workspace _Before this branch:_ * p50 time: 13.335 seconds * p90 time: 15.295 seconds _After this branch:_ * p50 time: 11.397 seconds * p90 time: 11.743 seconds ### Typical duration of deployment steps * Acquiring Deployment Lock: 1.096 seconds * Deployment Preparations and Operations: 1.477 seconds * Uploading Artifacts: 1.26 seconds * Finalizing Deployment: 9.699 seconds * Releasing Deployment Lock: 1.198 seconds --------- Co-authored-by: Pieter Noordhuis Co-authored-by: Andrew Nester --- bundle/config/deployment.go | 2 +- bundle/config/lock.go | 13 ++++++++++- bundle/config/mutator/process_target_mode.go | 22 ++++++++++++++++--- .../mutator/process_target_mode_test.go | 20 +++++++++++++++++ bundle/config/root.go | 12 ++++++++++ 5 files changed, 64 insertions(+), 5 deletions(-) diff --git a/bundle/config/deployment.go b/bundle/config/deployment.go index f89c7b3e..7f0f57a8 100644 --- a/bundle/config/deployment.go +++ b/bundle/config/deployment.go @@ -6,5 +6,5 @@ type Deployment struct { FailOnActiveRuns bool `json:"fail_on_active_runs,omitempty"` // Lock configures locking behavior on deployment. - Lock Lock `json:"lock" bundle:"readonly"` + Lock Lock `json:"lock"` } diff --git a/bundle/config/lock.go b/bundle/config/lock.go index 760099a9..10e9e1c9 100644 --- a/bundle/config/lock.go +++ b/bundle/config/lock.go @@ -1,7 +1,7 @@ package config type Lock struct { - // Enabled toggles deployment lock. True by default. + // Enabled toggles deployment lock. True by default except in development mode. // Use a pointer value so that only explicitly configured values are set // and we don't merge configuration with zero-initialized values. Enabled *bool `json:"enabled,omitempty"` @@ -11,9 +11,20 @@ type Lock struct { Force bool `json:"force,omitempty"` } +// IsEnabled checks if the deployment lock is enabled. func (lock Lock) IsEnabled() bool { if lock.Enabled != nil { return *lock.Enabled } return true } + +// IsExplicitlyEnabled checks if the deployment lock is explicitly enabled. +// Only returns true if locking is explicitly set using a command-line +// flag or configuration file. +func (lock Lock) IsExplicitlyEnabled() bool { + if lock.Enabled != nil { + return *lock.Enabled + } + return false +} diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index d3de5728..8e70fab7 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" @@ -29,9 +30,16 @@ func (m *processTargetMode) Name() string { // Mark all resources as being for 'development' purposes, i.e. // changing their their name, adding tags, and (in the future) // marking them as 'hidden' in the UI. -func transformDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { - r := b.Config.Resources +func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + if !b.Config.Bundle.Deployment.Lock.IsExplicitlyEnabled() { + log.Infof(ctx, "Development mode: disabling deployment lock since bundle.deployment.lock.enabled is not set to true") + err := disableDeploymentLock(b) + if err != nil { + return diag.FromErr(err) + } + } + r := b.Config.Resources shortName := b.Config.Workspace.CurrentUser.ShortName prefix := "[dev " + shortName + "] " @@ -100,6 +108,14 @@ func transformDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { return nil } +func disableDeploymentLock(b *bundle.Bundle) error { + return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.Map(v, "bundle.deployment.lock", func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "enabled", dyn.V(false)) + }) + }) +} + func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { if path := findNonUserPath(b); path != "" { return diag.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path) @@ -163,7 +179,7 @@ func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Di if diags != nil { return diags } - return transformDevelopmentMode(b) + return transformDevelopmentMode(ctx, b) case config.Production: isPrincipal := auth.IsServicePrincipal(b.Config.Workspace.CurrentUser.UserName) return validateProductionMode(ctx, b, isPrincipal) diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 17f83816..583efcfe 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -301,3 +301,23 @@ func TestAllResourcesRenamed(t *testing.T) { } } } + +func TestDisableLocking(t *testing.T) { + ctx := context.Background() + b := mockBundle(config.Development) + + err := transformDevelopmentMode(ctx, b) + require.Nil(t, err) + assert.False(t, b.Config.Bundle.Deployment.Lock.IsEnabled()) +} + +func TestDisableLockingDisabled(t *testing.T) { + ctx := context.Background() + b := mockBundle(config.Development) + explicitlyEnabled := true + b.Config.Bundle.Deployment.Lock.Enabled = &explicitlyEnabled + + err := transformDevelopmentMode(ctx, b) + require.Nil(t, err) + assert.True(t, b.Config.Bundle.Deployment.Lock.IsEnabled(), "Deployment lock should remain enabled in development mode when explicitly enabled") +} diff --git a/bundle/config/root.go b/bundle/config/root.go index 18b548d6..70ca14ea 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -143,6 +143,18 @@ func (r *Root) updateWithDynamicValue(nv dyn.Value) error { return nil } +// Mutate applies a transformation to the dynamic configuration value of a Root object. +// +// Parameters: +// - fn: A function that mutates a dyn.Value object +// +// Example usage, setting bundle.deployment.lock.enabled to false: +// +// err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +// return dyn.Map(v, "bundle.deployment.lock", func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { +// return dyn.Set(v, "enabled", dyn.V(false)) +// }) +// }) func (r *Root) Mutate(fn func(dyn.Value) (dyn.Value, error)) error { err := r.initializeDynamicValue() if err != nil {