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 <pcnoordhuis@gmail.com>
Co-authored-by: Andrew Nester <andrew.nester.dev@gmail.com>
This commit is contained in:
Lennart Kats (databricks) 2024-04-17 18:59:39 -07:00 committed by GitHub
parent 77d6820075
commit c3a7d17d1d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 64 additions and 5 deletions

View File

@ -6,5 +6,5 @@ type Deployment struct {
FailOnActiveRuns bool `json:"fail_on_active_runs,omitempty"` FailOnActiveRuns bool `json:"fail_on_active_runs,omitempty"`
// Lock configures locking behavior on deployment. // Lock configures locking behavior on deployment.
Lock Lock `json:"lock" bundle:"readonly"` Lock Lock `json:"lock"`
} }

View File

@ -1,7 +1,7 @@
package config package config
type Lock struct { 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 // Use a pointer value so that only explicitly configured values are set
// and we don't merge configuration with zero-initialized values. // and we don't merge configuration with zero-initialized values.
Enabled *bool `json:"enabled,omitempty"` Enabled *bool `json:"enabled,omitempty"`
@ -11,9 +11,20 @@ type Lock struct {
Force bool `json:"force,omitempty"` Force bool `json:"force,omitempty"`
} }
// IsEnabled checks if the deployment lock is enabled.
func (lock Lock) IsEnabled() bool { func (lock Lock) IsEnabled() bool {
if lock.Enabled != nil { if lock.Enabled != nil {
return *lock.Enabled return *lock.Enabled
} }
return true 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
}

View File

@ -9,6 +9,7 @@ import (
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/auth"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/log"
"github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/ml" "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. // Mark all resources as being for 'development' purposes, i.e.
// changing their their name, adding tags, and (in the future) // changing their their name, adding tags, and (in the future)
// marking them as 'hidden' in the UI. // marking them as 'hidden' in the UI.
func transformDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
r := b.Config.Resources 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 shortName := b.Config.Workspace.CurrentUser.ShortName
prefix := "[dev " + shortName + "] " prefix := "[dev " + shortName + "] "
@ -100,6 +108,14 @@ func transformDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
return nil 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 { func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
if path := findNonUserPath(b); path != "" { if path := findNonUserPath(b); path != "" {
return diag.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", 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 { if diags != nil {
return diags return diags
} }
return transformDevelopmentMode(b) return transformDevelopmentMode(ctx, b)
case config.Production: case config.Production:
isPrincipal := auth.IsServicePrincipal(b.Config.Workspace.CurrentUser.UserName) isPrincipal := auth.IsServicePrincipal(b.Config.Workspace.CurrentUser.UserName)
return validateProductionMode(ctx, b, isPrincipal) return validateProductionMode(ctx, b, isPrincipal)

View File

@ -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")
}

View File

@ -143,6 +143,18 @@ func (r *Root) updateWithDynamicValue(nv dyn.Value) error {
return nil 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 { func (r *Root) Mutate(fn func(dyn.Value) (dyn.Value, error)) error {
err := r.initializeDynamicValue() err := r.initializeDynamicValue()
if err != nil { if err != nil {