diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 5949e2ebc5d..0f905348d49 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -628,6 +628,11 @@ func runTest(t *testing.T, cmd.Env = append(cmd.Env, "GOCOVERDIR="+coverDir) } + // Set unique cache folder for this test to avoid race conditions between parallel tests + // Use test temp directory to avoid polluting user's cache + uniqueCacheDir := filepath.Join(t.TempDir(), ".cache") + cmd.Env = append(cmd.Env, "DATABRICKS_CACHE_DIR="+uniqueCacheDir) + for _, key := range utils.SortedKeys(config.Env) { if hasKey(customEnv, key) { // We want EnvMatrix to take precedence. diff --git a/acceptance/bundle/resource_deps/job_tasks/out.telemetry.direct.txt b/acceptance/bundle/resource_deps/job_tasks/out.telemetry.direct.txt index 30f909c79dd..410528fdf3f 100644 --- a/acceptance/bundle/resource_deps/job_tasks/out.telemetry.direct.txt +++ b/acceptance/bundle/resource_deps/job_tasks/out.telemetry.direct.txt @@ -2,6 +2,8 @@ experimental.use_legacy_run_as false has_classic_interactive_compute false has_classic_job_compute false has_serverless_compute true +local.cache.attempt true +local.cache.miss true presets_name_prefix_is_set false python_wheel_wrapper_is_set false resref_jobs.tags.* true diff --git a/acceptance/bundle/resource_deps/job_tasks/out.telemetry.terraform.txt b/acceptance/bundle/resource_deps/job_tasks/out.telemetry.terraform.txt index 38ee5032b11..50371a06442 100644 --- a/acceptance/bundle/resource_deps/job_tasks/out.telemetry.terraform.txt +++ b/acceptance/bundle/resource_deps/job_tasks/out.telemetry.terraform.txt @@ -2,6 +2,8 @@ experimental.use_legacy_run_as false has_classic_interactive_compute false has_classic_job_compute false has_serverless_compute true +local.cache.attempt true +local.cache.miss true presets_name_prefix_is_set false python_wheel_wrapper_is_set false resref_jobs.tags.* true diff --git a/acceptance/bundle/resource_deps/resources_var/output.txt b/acceptance/bundle/resource_deps/resources_var/output.txt index 45434a5658d..cd34790c2ca 100644 --- a/acceptance/bundle/resource_deps/resources_var/output.txt +++ b/acceptance/bundle/resource_deps/resources_var/output.txt @@ -40,6 +40,8 @@ experimental.use_legacy_run_as false has_classic_interactive_compute false has_classic_job_compute false has_serverless_compute false +local.cache.attempt true +local.cache.hit true presets_name_prefix_is_set true python_wheel_wrapper_is_set false resref_volumes.catalog_name true diff --git a/acceptance/bundle/run/scripts/databricks-cli/test.toml b/acceptance/bundle/run/scripts/databricks-cli/test.toml index 24cf889ece1..e42dd928e61 100644 --- a/acceptance/bundle/run/scripts/databricks-cli/test.toml +++ b/acceptance/bundle/run/scripts/databricks-cli/test.toml @@ -1,6 +1,9 @@ RecordRequests = true IncludeRequestHeaders = ["Authorization"] +[Env] +DATABRICKS_CACHE_ENABLED = 'false' + # "client_id:client_secret" in base64 is Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ=, expect to # see this in Authorization header [[Repls]] diff --git a/acceptance/bundle/telemetry/deploy-compute-type/output.txt b/acceptance/bundle/telemetry/deploy-compute-type/output.txt index f6376bf9dd3..a424df8ce1d 100644 --- a/acceptance/bundle/telemetry/deploy-compute-type/output.txt +++ b/acceptance/bundle/telemetry/deploy-compute-type/output.txt @@ -13,6 +13,14 @@ Deployment complete! >>> cat out.requests.txt [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.miss", + "value": true + }, { "key": "experimental.use_legacy_run_as", "value": false @@ -47,6 +55,14 @@ Deployment complete! } ] [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.hit", + "value": true + }, { "key": "experimental.use_legacy_run_as", "value": false diff --git a/acceptance/bundle/telemetry/deploy-experimental/output.txt b/acceptance/bundle/telemetry/deploy-experimental/output.txt index 437a3c6f9e9..05bc64e441d 100644 --- a/acceptance/bundle/telemetry/deploy-experimental/output.txt +++ b/acceptance/bundle/telemetry/deploy-experimental/output.txt @@ -12,6 +12,14 @@ Deployment complete! >>> cat out.requests.txt { "bool_values": [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.miss", + "value": true + }, { "key": "experimental.use_legacy_run_as", "value": true diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt b/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt index 567b4282000..31ff8e9cf7e 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt +++ b/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt @@ -8,6 +8,14 @@ Deployment complete! >>> cat out.requests.txt { "bool_values": [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.miss", + "value": true + }, { "key": "experimental.use_legacy_run_as", "value": false diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt index 7a710b9045b..39b671bec32 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt +++ b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt @@ -8,6 +8,14 @@ Deployment complete! >>> cat out.requests.txt { "bool_values": [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.miss", + "value": true + }, { "key": "experimental.use_legacy_run_as", "value": false diff --git a/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt b/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt index 207ee71d24c..a9b8ce4ae6e 100644 --- a/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt +++ b/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt @@ -16,6 +16,14 @@ Deployment complete! >>> cat out.requests.txt { "bool_values": [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.miss", + "value": true + }, { "key": "artifact_build_command_is_set", "value": false @@ -48,6 +56,14 @@ Deployment complete! } { "bool_values": [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.hit", + "value": true + }, { "key": "artifact_build_command_is_set", "value": true diff --git a/acceptance/bundle/telemetry/deploy/out.telemetry.txt b/acceptance/bundle/telemetry/deploy/out.telemetry.txt index 57b9b46f855..f945233dd16 100644 --- a/acceptance/bundle/telemetry/deploy/out.telemetry.txt +++ b/acceptance/bundle/telemetry/deploy/out.telemetry.txt @@ -42,6 +42,14 @@ "lookup_variable_count": 0, "target_count": 1, "bool_values": [ + { + "key": "local.cache.attempt", + "value": true + }, + { + "key": "local.cache.miss", + "value": true + }, { "key": "experimental.use_legacy_run_as", "value": false @@ -76,7 +84,8 @@ } ], "bundle_mode": "TYPE_UNSPECIFIED", - "workspace_artifact_path_type": "WORKSPACE_FILE_SYSTEM" + "workspace_artifact_path_type": "WORKSPACE_FILE_SYSTEM", + "local_cache_measurements_ms": [...redacted...] } } } diff --git a/acceptance/bundle/telemetry/test.toml b/acceptance/bundle/telemetry/test.toml index d583d9d69b0..d47cfd33e3d 100644 --- a/acceptance/bundle/telemetry/test.toml +++ b/acceptance/bundle/telemetry/test.toml @@ -1,6 +1,9 @@ RecordRequests = true IncludeRequestHeaders = ["User-Agent"] +[Env] +DATABRICKS_CACHE_ENABLED = 'false' + [[Repls]] Old = '"execution_time_ms": \d{1,5},' New = '"execution_time_ms": SMALL_INT,' @@ -8,3 +11,7 @@ New = '"execution_time_ms": SMALL_INT,' [[Repls]] Old = '(linux|darwin|windows)' New = '[OS]' + +[[Repls]] +Old = '"local_cache_measurements_ms": \[[^\]]*\]' +New = '"local_cache_measurements_ms": [...redacted...]' diff --git a/acceptance/bundle/user_agent/test.toml b/acceptance/bundle/user_agent/test.toml index 2bb1fd7f661..9295fedc55e 100644 --- a/acceptance/bundle/user_agent/test.toml +++ b/acceptance/bundle/user_agent/test.toml @@ -1,3 +1,6 @@ RecordRequests = true Local = true IncludeRequestHeaders = ["User-Agent"] + +[Env] +DATABRICKS_CACHE_ENABLED = 'false' diff --git a/acceptance/cache/clear/databricks.yml b/acceptance/cache/clear/databricks.yml new file mode 100644 index 00000000000..5557aa4ad43 --- /dev/null +++ b/acceptance/cache/clear/databricks.yml @@ -0,0 +1,2 @@ +bundle: + name: cache-clear-test diff --git a/acceptance/cache/clear/out.test.toml b/acceptance/cache/clear/out.test.toml new file mode 100644 index 00000000000..d560f1de043 --- /dev/null +++ b/acceptance/cache/clear/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cache/clear/output.txt b/acceptance/cache/clear/output.txt new file mode 100644 index 00000000000..bba37b6ccb0 --- /dev/null +++ b/acceptance/cache/clear/output.txt @@ -0,0 +1,19 @@ + +=== First call in a session is expected to be a cache miss: +[DEBUG_TIMESTAMP] Debug: [Local Cache] using cache key: [SHA256_HASH] +[DEBUG_TIMESTAMP] Debug: [Local Cache] failed to stat cache file: (redacted) +[DEBUG_TIMESTAMP] Debug: [Local Cache] cache miss, computing +[DEBUG_TIMESTAMP] Debug: [Local Cache] computed and stored result + +=== Second call in a session is expected to be a cache hit +[DEBUG_TIMESTAMP] Debug: [Local Cache] using cache key: [SHA256_HASH] +[DEBUG_TIMESTAMP] Debug: [Local Cache] cache hit + +>>> [CLI] cache clear +Cache cleared successfully from [TEST_TMP_DIR]/.cache + +=== First call after a clear is expected to be a cache miss: +[DEBUG_TIMESTAMP] Debug: [Local Cache] using cache key: [SHA256_HASH] +[DEBUG_TIMESTAMP] Debug: [Local Cache] failed to stat cache file: (redacted) +[DEBUG_TIMESTAMP] Debug: [Local Cache] cache miss, computing +[DEBUG_TIMESTAMP] Debug: [Local Cache] computed and stored result diff --git a/acceptance/cache/clear/script b/acceptance/cache/clear/script new file mode 100644 index 00000000000..7c082ae82dd --- /dev/null +++ b/acceptance/cache/clear/script @@ -0,0 +1,16 @@ +export DATABRICKS_CACHE_DIR=$(pwd)/.cache + +$CLI cache clear &> /dev/null + +title "First call in a session is expected to be a cache miss:\n" +trace $CLI bundle validate --debug 2>&1 | grep "Local Cache" | grep -v "cache path" + +title "Second call in a session is expected to be a cache hit\n" +trace $CLI bundle validate --debug 2>&1 | grep "Local Cache" | grep -v "cache path" + +trace $CLI cache clear + +title "First call after a clear is expected to be a cache miss:\n" +trace $CLI bundle validate --debug 2>&1 | grep "Local Cache" | grep -v "cache path" + +rm -rf "${DATABRICKS_CACHE_DIR}" diff --git a/acceptance/cache/clear/test.toml b/acceptance/cache/clear/test.toml new file mode 100644 index 00000000000..0b1b2fe5e7e --- /dev/null +++ b/acceptance/cache/clear/test.toml @@ -0,0 +1,14 @@ +Cloud = false +Local = true + +[Env] +DATABRICKS_CACHE_ENABLED = 'true' + +# Redact structured logging fields from debug output +[[Repls]] +Old = ' pid=[0-9]+' +New = '' + +[[Repls]] +Old = ' mutator=[A-Za-z]+' +New = '' diff --git a/acceptance/cache/simple/databricks.yml b/acceptance/cache/simple/databricks.yml new file mode 100644 index 00000000000..79d36f1e342 --- /dev/null +++ b/acceptance/cache/simple/databricks.yml @@ -0,0 +1,2 @@ +bundle: + name: exploratory-cache-test diff --git a/acceptance/cache/simple/out.test.toml b/acceptance/cache/simple/out.test.toml new file mode 100644 index 00000000000..d560f1de043 --- /dev/null +++ b/acceptance/cache/simple/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cache/simple/output.txt b/acceptance/cache/simple/output.txt new file mode 100644 index 00000000000..093900b94b7 --- /dev/null +++ b/acceptance/cache/simple/output.txt @@ -0,0 +1,21 @@ + +=== First call in a session is expected to be a cache miss: +[DEBUG_TIMESTAMP] Debug: [Local Cache] using cache key: [SHA256_HASH] +[DEBUG_TIMESTAMP] Debug: [Local Cache] failed to stat cache file: (redacted) +[DEBUG_TIMESTAMP] Debug: [Local Cache] cache miss, computing +[DEBUG_TIMESTAMP] Debug: [Local Cache] computed and stored result + +=== Second call in a session is expected to be a cache hit +[DEBUG_TIMESTAMP] Debug: [Local Cache] using cache key: [SHA256_HASH] +[DEBUG_TIMESTAMP] Debug: [Local Cache] cache hit + +=== Bundle deploy should send telemetry values + +>>> [CLI] bundle deploy -p dogfood +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/exploratory-cache-test/default/files... +Deploying resources... +Deployment complete! + +>>> print_telemetry_bool_values +local.cache.attempt true +local.cache.hit true diff --git a/acceptance/cache/simple/script b/acceptance/cache/simple/script new file mode 100644 index 00000000000..a2907174bf3 --- /dev/null +++ b/acceptance/cache/simple/script @@ -0,0 +1,16 @@ +export DATABRICKS_CACHE_DIR=$(pwd)/.cache + +$CLI cache clear &> /dev/null + +title "First call in a session is expected to be a cache miss:\n" +trace $CLI bundle validate -p dogfood --debug 2>&1 | grep "Local Cache" | grep -v "cache path" + +title "Second call in a session is expected to be a cache hit\n" +trace $CLI bundle validate -p dogfood --debug 2>&1 | grep "Local Cache" | grep -v "cache path" + +title "Bundle deploy should send telemetry values\n" +trace $CLI bundle deploy -p dogfood + +trace print_telemetry_bool_values | grep "local.cache" +rm out.requests.txt +rm -rf "${DATABRICKS_CACHE_DIR}" diff --git a/acceptance/cache/simple/test.toml b/acceptance/cache/simple/test.toml new file mode 100644 index 00000000000..07f6a81177e --- /dev/null +++ b/acceptance/cache/simple/test.toml @@ -0,0 +1,16 @@ +Cloud = false +Local = true + +RecordRequests = true + +[Env] +DATABRICKS_CACHE_ENABLED = 'true' + +# Redact structured logging fields from debug output +[[Repls]] +Old = ' pid=[0-9]+' +New = '' + +[[Repls]] +Old = ' mutator=[A-Za-z]+' +New = '' diff --git a/acceptance/cache/test.toml b/acceptance/cache/test.toml new file mode 100644 index 00000000000..ef8415db362 --- /dev/null +++ b/acceptance/cache/test.toml @@ -0,0 +1,15 @@ +[[Repls]] +Old = '\d\d:\d\d:\d\d' +New = "[DEBUG_TIMESTAMP]" + +[[Repls]] +Old = '[a-f0-9]{64}' +New = "[SHA256_HASH]" + +[[Repls]] +Old = '[a-f0-9]{12}' +New = "[CACHE_KEY]" + +[[Repls]] +Old = 'failed to stat cache file: .*' +New = "failed to stat cache file: (redacted)" diff --git a/acceptance/help/output.txt b/acceptance/help/output.txt index 7f2f7426a91..50b740c3116 100644 --- a/acceptance/help/output.txt +++ b/acceptance/help/output.txt @@ -158,6 +158,7 @@ Additional Commands: account Databricks Account Commands api Perform Databricks API call auth Authentication related commands + cache Local cache related commands completion Generate the autocompletion script for the specified shell configure Configure authentication help Help about any command diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 5f3d0309478..2129fa55737 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -213,6 +213,8 @@ func LoadConfig(t *testing.T, dir string) (TestConfig, string) { } } + // Always ignore .cache directory (used by local cache) + result.Ignore = append(result.Ignore, ".cache") result.CompiledIgnoreObject = ignore.CompileIgnoreLines(result.Ignore...) return result, strings.Join(configs, ", ") diff --git a/bundle/bundle.go b/bundle/bundle.go index dca26fc559d..837521f03f3 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -13,12 +13,14 @@ import ( "os" "path/filepath" "sync" + "time" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/direct" "github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/metadata" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/cache" "github.com/databricks/cli/libs/fileset" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" @@ -50,6 +52,7 @@ type Metrics struct { PythonAddedResourcesCount int64 PythonUpdatedResourcesCount int64 ExecutionTimes []protos.IntMapEntry + LocalCacheMeasurementsMs []protos.IntMapEntry // Local cache measurements stored as milliseconds } // SetBoolValue sets the value of a boolean metric. @@ -70,6 +73,13 @@ func (m *Metrics) AddBoolValue(key string, value bool) { m.BoolValues = append(m.BoolValues, protos.BoolMapEntry{Key: key, Value: value}) } +// AddDurationValue sets the value of a duration metric in milliseconds. +// The value is added to the list of measurements. +func (m *Metrics) AddDurationValue(key string, value time.Duration) { + valueMs := value.Milliseconds() + m.LocalCacheMeasurementsMs = append(m.LocalCacheMeasurementsMs, protos.IntMapEntry{Key: key, Value: valueMs}) +} + type Bundle struct { // BundleRootPath is the local path to the root directory of the bundle. // It is set when we instantiate a new bundle instance. @@ -140,6 +150,10 @@ type Bundle struct { // The implementation depends on the cloud being targeted. Tagging tags.Cloud + // Cache is used for caching API responses (e.g., current user). + // By default, operates in measurement-only mode. Set DATABRICKS_CACHE_ENABLED=true to enable actual caching. + Cache *cache.Cache + Metrics Metrics } diff --git a/bundle/config/mutator/initialize_cache.go b/bundle/config/mutator/initialize_cache.go new file mode 100644 index 00000000000..d27e52efc12 --- /dev/null +++ b/bundle/config/mutator/initialize_cache.go @@ -0,0 +1,27 @@ +package mutator + +import ( + "context" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/cache" + "github.com/databricks/cli/libs/diag" +) + +type initializeCache struct{} + +// InitializeCache initializes the bundle cache which can be used to cache API responses. +func InitializeCache() bundle.Mutator { + return &initializeCache{} +} + +func (m *initializeCache) Name() string { + return "InitializeCache" +} + +func (m *initializeCache) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // Initialize cache with 30 minute expiry for user information + b.Cache = cache.NewCache(ctx, "user", 30*time.Minute, &b.Metrics) + return nil +} diff --git a/bundle/config/mutator/initialize_cache_test.go b/bundle/config/mutator/initialize_cache_test.go new file mode 100644 index 00000000000..f6381fd906c --- /dev/null +++ b/bundle/config/mutator/initialize_cache_test.go @@ -0,0 +1,26 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestInitializeCache(t *testing.T) { + ctx := context.Background() + b := &bundle.Bundle{} + + // Cache should be nil initially + assert.Nil(t, b.Cache) + + // Apply the mutator + diags := bundle.Apply(ctx, b, mutator.InitializeCache()) + require.NoError(t, diags.Error()) + + // Cache should now be initialized + assert.NotNil(t, b.Cache) +} diff --git a/bundle/config/mutator/populate_current_user.go b/bundle/config/mutator/populate_current_user.go index 1e7fae629e8..0088a024516 100644 --- a/bundle/config/mutator/populate_current_user.go +++ b/bundle/config/mutator/populate_current_user.go @@ -5,9 +5,11 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/cache" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/iamutil" "github.com/databricks/cli/libs/tags" + "github.com/databricks/databricks-sdk-go/service/iam" ) type populateCurrentUser struct{} @@ -25,9 +27,12 @@ func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) diag. if b.Config.Workspace.CurrentUser != nil { return nil } - w := b.WorkspaceClient() - me, err := w.CurrentUser.Me(ctx) + + fingerprint := b.GetUserFingerprint(ctx) + me, err := cache.GetOrCompute(ctx, b.Cache, fingerprint, func(ctx context.Context) (*iam.User, error) { + return w.CurrentUser.Me(ctx) + }) if err != nil { return diag.FromErr(err) } diff --git a/bundle/fingerprint.go b/bundle/fingerprint.go new file mode 100644 index 00000000000..526547b0ab5 --- /dev/null +++ b/bundle/fingerprint.go @@ -0,0 +1,34 @@ +package bundle + +import ( + "context" + "net/http" +) + +type UserFingerprint struct { + Host string `json:"host"` + AuthHeader string `json:"auth_header"` +} + +func (f *UserFingerprint) IsEmpty() bool { + return f.Host == "" && f.AuthHeader == "" +} + +func (b *Bundle) GetUserFingerprint(ctx context.Context) UserFingerprint { + return UserFingerprint{ + Host: b.WorkspaceClient().Config.Host, + AuthHeader: b.getAuthorizationHeader(), + } +} + +// getAuthorizationHeader extracts the Authorization header from the workspace client configuration. +// If it fails to authenticate, it returns an empty string. +func (b *Bundle) getAuthorizationHeader() string { + // Create a dummy request to extract the Authorization header + req := &http.Request{Header: http.Header{}} + if err := b.WorkspaceClient().Config.Authenticate(req); err != nil { + return "" + } + + return req.Header.Get("Authorization") +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index cfacba015a7..761714b48e6 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -50,7 +50,12 @@ func Initialize(ctx context.Context, b *bundle.Bundle) { // Updates (typed) b.Config.{Sync,Include,Exclude} they set to be relative to SyncRootPath instead of bundle root mutator.SyncInferRoot(), + // Updates (typed): b.Cache (initializes cache for API responses) + // Initialize cache before any mutator that might need it + mutator.InitializeCache(), + // Reads (typed): b.Config.Workspace.CurrentUser (checks if it's already set) + // Reads (typed): b.Cache (uses cache for current user API call) // Updates (typed): b.Config.Workspace.CurrentUser (sets user information from API) // Updates (typed): b.Tagging (configures tagging object based on current cloud) mutator.PopulateCurrentUser(), diff --git a/bundle/phases/telemetry.go b/bundle/phases/telemetry.go index f49f55ec845..4584e9fc5e1 100644 --- a/bundle/phases/telemetry.go +++ b/bundle/phases/telemetry.go @@ -175,6 +175,7 @@ func logDeployTelemetry(ctx context.Context, b *bundle.Bundle) { TargetCount: b.Metrics.TargetCount, WorkspaceArtifactPathType: artifactPathType, BoolValues: b.Metrics.BoolValues, + LocalCacheMeasurementsMs: b.Metrics.LocalCacheMeasurementsMs, PythonAddedResourcesCount: b.Metrics.PythonAddedResourcesCount, PythonUpdatedResourcesCount: b.Metrics.PythonUpdatedResourcesCount, PythonResourceLoadersCount: int64(len(experimentalConfig.Python.Resources)), diff --git a/cmd/cache/cache.go b/cmd/cache/cache.go new file mode 100644 index 00000000000..88dec62acc6 --- /dev/null +++ b/cmd/cache/cache.go @@ -0,0 +1,42 @@ +package cache + +import ( + "github.com/databricks/cli/libs/cache" + "github.com/spf13/cobra" +) + +func New() *cobra.Command { + cmd := &cobra.Command{ + Use: "cache", + Short: "Local cache related commands", + Long: "Manage local cache used by the Databricks CLI for improved performance", + } + + cmd.AddCommand(newClearCommand()) + return cmd +} + +func newClearCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "clear", + Short: "Clear all local cache files", + Long: `Remove all cached files stored locally by the Databricks CLI. + +This clears the cache for all CLI versions, not just the current version. +The cache directory is typically located at: + - Linux/macOS: ~/.cache/databricks/ + - Windows: %LOCALAPPDATA%\databricks\ + +You can override this with the DATABRICKS_CACHE_DIR environment variable.`, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + cachePath, err := cache.ClearFileCache(ctx) + if err != nil { + return err + } + cmd.Printf("Cache cleared successfully from %s\n", cachePath) + return nil + }, + } + return cmd +} diff --git a/cmd/cmd.go b/cmd/cmd.go index f697af17e2d..459b4284c2e 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/cmd/api" "github.com/databricks/cli/cmd/auth" "github.com/databricks/cli/cmd/bundle" + "github.com/databricks/cli/cmd/cache" "github.com/databricks/cli/cmd/configure" "github.com/databricks/cli/cmd/experimental" "github.com/databricks/cli/cmd/fs" @@ -86,6 +87,7 @@ func New(ctx context.Context) *cobra.Command { cli.AddCommand(api.New()) cli.AddCommand(auth.New()) cli.AddCommand(bundle.New()) + cli.AddCommand(cache.New()) cli.AddCommand(experimental.New()) cli.AddCommand(psql.New()) cli.AddCommand(configure.New()) diff --git a/internal/build/info.go b/internal/build/info.go index 8ddf4d4f2e6..15967be8ff4 100644 --- a/internal/build/info.go +++ b/internal/build/info.go @@ -4,6 +4,7 @@ import ( "fmt" "runtime/debug" "strconv" + "strings" "sync" "time" @@ -29,6 +30,18 @@ type Info struct { BuildTime time.Time } +// GetSanitizedVersion removes characters from version string that might be problematic in file paths. +// Particularly important for Windows which has restrictions on certain characters. +func (i Info) GetSanitizedVersion() string { + // Replace + with - (used in version metadata like "1.0.0+abc123") + version := strings.ReplaceAll(i.Version, "+", "-") + // Remove any other potentially problematic characters + version = strings.ReplaceAll(version, ":", "-") + version = strings.ReplaceAll(version, "/", "-") + version = strings.ReplaceAll(version, "\\", "-") + return version +} + var info Info var once sync.Once diff --git a/internal/build/info_test.go b/internal/build/info_test.go index 1ae94fbce75..7b33a114e86 100644 --- a/internal/build/info_test.go +++ b/internal/build/info_test.go @@ -2,8 +2,57 @@ package build import ( "testing" + + "github.com/stretchr/testify/assert" ) func TestGetDetails(t *testing.T) { GetInfo() } + +func TestGetSanitizedVersion(t *testing.T) { + tests := []struct { + name string + version string + expected string + }{ + { + name: "version with plus", + version: "1.0.0+abc123", + expected: "1.0.0-abc123", + }, + { + name: "version with colon (Windows problematic)", + version: "1.0.0:dev", + expected: "1.0.0-dev", + }, + { + name: "version with forward slash (Windows problematic)", + version: "1.0.0/beta", + expected: "1.0.0-beta", + }, + { + name: "version with backslash (Windows problematic)", + version: "1.0.0\\test", + expected: "1.0.0-test", + }, + { + name: "version with multiple problematic characters", + version: "1.0.0+abc:123/test\\dev", + expected: "1.0.0-abc-123-test-dev", + }, + { + name: "clean version", + version: "1.0.0-dev", + expected: "1.0.0-dev", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + info := Info{Version: tt.version} + result := info.GetSanitizedVersion() + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/libs/cache/cache.go b/libs/cache/cache.go new file mode 100644 index 00000000000..513f7ebd002 --- /dev/null +++ b/libs/cache/cache.go @@ -0,0 +1,58 @@ +package cache + +import ( + "context" + "encoding/json" + + "github.com/databricks/cli/libs/log" +) + +// cacheImpl is the internal interface for cache implementations. +type cacheImpl interface { + // getOrComputeJSON retrieves cached JSON bytes or computes them. + // The compute function must return JSON-encoded data as []byte. + // The returned []byte is also expected to be JSON-encoded. + getOrComputeJSON(ctx context.Context, fingerprint any, compute func(ctx context.Context) ([]byte, error)) ([]byte, error) +} + +// Cache provides a concrete cache that works with any type through the generic GetOrCompute function. +// Create with NewCache() and use GetOrCompute[T]() for type-safe caching. +type Cache struct { + impl cacheImpl +} + +// GetOrCompute retrieves cached content for the given fingerprint, or computes it using the provided function. +// If the content is found in cache, it is returned directly. +// If not found, the compute function is called, its result is cached, and then returned. +// The fingerprint can be any struct that will be serialized deterministically for cache key generation. +// Cache operations fail open: if caching fails, the compute function is still called. +// Returns an error only if the compute function fails. +// The type parameter T is inferred from the compute function's return type. +func GetOrCompute[T any](ctx context.Context, c *Cache, fingerprint any, compute func(ctx context.Context) (T, error)) (T, error) { + var zero T + + // Wrap the compute function to serialize to JSON + computeJSON := func(ctx context.Context) ([]byte, error) { + result, err := compute(ctx) + if err != nil { + return nil, err + } + return json.Marshal(result) + } + + // Call the internal method + jsonBytes, err := c.impl.getOrComputeJSON(ctx, fingerprint, computeJSON) + if err != nil { + return zero, err + } + + // Unmarshal into the target type + var result T + if err := json.Unmarshal(jsonBytes, &result); err != nil { + // Fail open: if cached data is corrupted, log and recompute + log.Debugf(ctx, "[Local Cache] failed to unmarshal cached data, recomputing: %v", err) + return compute(ctx) + } + + return result, nil +} diff --git a/libs/cache/file_cache.go b/libs/cache/file_cache.go new file mode 100644 index 00000000000..deb9780b65f --- /dev/null +++ b/libs/cache/file_cache.go @@ -0,0 +1,272 @@ +package cache + +import ( + "context" + "fmt" + "os" + "path/filepath" + "sync" + "time" + + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" +) + +// Metrics is a local interface for tracking cache telemetry. +type Metrics interface { + SetBoolValue(key string, value bool) + AddDurationValue(key string, value time.Duration) +} + +// fileCache implements the cacheImpl interface using local disk storage. +type fileCache struct { + baseDir string + expiry time.Duration + mu sync.Mutex + metrics Metrics + cacheEnabled bool // If true, cached values are returned; if false, cache is only used for measurement +} + +// newFileCacheWithBaseDir creates a new file-based cache that stores data in the specified directory. +func newFileCacheWithBaseDir(ctx context.Context, baseDir string, expiry time.Duration) (*fileCache, error) { + if err := os.MkdirAll(baseDir, 0o700); err != nil { + return nil, fmt.Errorf("failed to create cache directory: %w", err) + } + + fc := &fileCache{ + baseDir: baseDir, + expiry: expiry, + } + + // Clean up expired files synchronously + fc.cleanupExpiredFiles(ctx) + + return fc, nil +} + +// isExpired checks if a file with the given modification time has expired. +func (fc *fileCache) isExpired(modTime time.Time) bool { + expiryThreshold := time.Now().Add(-fc.expiry) + return modTime.Before(expiryThreshold) +} + +// cleanupExpiredFiles removes expired cache files from disk based on file modification time. +// This runs synchronously once when the cache is created. +// Files older than expiry duration are deleted. +func (fc *fileCache) cleanupExpiredFiles(ctx context.Context) { + err := filepath.Walk(fc.baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + // Log walk errors but continue cleanup + log.Debugf(ctx, "[Local Cache] cleanup: failed to access path %s: %v", path, err) + return nil + } + + if info.IsDir() { + return nil + } + + // Remove any leftover .tmp files (from failed atomic writes) + if filepath.Ext(info.Name()) == ".tmp" { + _ = os.Remove(path) + return nil + } + + // Only process .json cache files + if filepath.Ext(info.Name()) != ".json" { + return nil + } + + // Check if file is expired based on modification time + if fc.isExpired(info.ModTime()) { + if err := os.Remove(path); err != nil { + log.Tracef(ctx, "[Local Cache] cleanup: failed to remove expired file %s: %v", path, err) + } else { + log.Tracef(ctx, "[Local Cache] cleanup: removed expired file %s", path) + } + } + + return nil + }) + if err != nil { + log.Debugf(ctx, "[Local Cache] cleanup: failed to walk cache directory: %v", err) + } +} + +func getCacheBaseDir(ctx context.Context) (string, error) { + // Check if user has configured a custom cache directory + if customCacheDir := env.Get(ctx, "DATABRICKS_CACHE_DIR"); customCacheDir != "" { + return customCacheDir, nil + } + + // Use default cache directory + userCacheDir, err := os.UserCacheDir() + if err != nil { + return "", fmt.Errorf("failed to get user cache directory: %w", err) + } + return filepath.Join(userCacheDir, "databricks"), nil +} + +// NewCache creates a new file-based cache using UserCacheDir() + "databricks" + version + cached component name. +// Including the CLI version in the path ensures cache isolation across different CLI versions. +// By default, the cache operates in measurement-only mode (cacheEnabled=false), which means it will: +// - Check if cached values exist +// - Measure how much time would have been saved +// - Emit metrics about potential savings +// - Always compute the value (never actually use the cache) +// Set DATABRICKS_CACHE_ENABLED=true to enable actual caching. +// The returned cache can handle multiple types through the generic GetOrCompute function. +func NewCache(ctx context.Context, component string, expiry time.Duration, metrics Metrics) *Cache { + cacheBaseDir, err := getCacheBaseDir(ctx) + if err != nil { + return &Cache{impl: &noopFileCache{}} + } + + // Include CLI version in cache path to avoid issues across versions + // Sanitize version string for use in file paths + version := build.GetInfo().GetSanitizedVersion() + baseDir := filepath.Join(cacheBaseDir, version, component) + fc, err := newFileCacheWithBaseDir(ctx, baseDir, expiry) + if err != nil { + return &Cache{impl: &noopFileCache{}} + } + fc.metrics = metrics + + // Check if cache is enabled; default is false (measurement-only mode) + // Only "true" enables caching; any other value (including "false", "1", etc.) keeps it disabled + fc.cacheEnabled = env.Get(ctx, "DATABRICKS_CACHE_ENABLED") == "true" + return &Cache{impl: fc} +} + +func (fc *fileCache) addTelemetryMetric(key string) { + if fc.metrics != nil { + fc.metrics.SetBoolValue(key, true) + } +} + +// getOrComputeJSON retrieves cached content or computes it using the provided function. +// Cache operations fail open: if caching fails, the compute function is still called. +// When cacheEnabled is false, the cache checks if values exist and measures potential time savings, +// but always computes and never returns cached values. +func (fc *fileCache) getOrComputeJSON(ctx context.Context, fingerprint any, compute func(ctx context.Context) ([]byte, error)) ([]byte, error) { + // Convert fingerprint to deterministic hash - this is our cache key + cacheKey, err := fingerprintToHash(fingerprint) + if err != nil { + // Fail open: if we can't generate cache key, just compute directly + log.Debugf(ctx, "[Local Cache] failed to generate cache key, computing without cache: %v", err) + return compute(ctx) + } + + log.Debugf(ctx, "[Local Cache] using cache key: %s", cacheKey) + fc.addTelemetryMetric("local.cache.attempt") + + cachePath := fc.getCachePath(cacheKey) + + // Acquire lock to prevent concurrent and double computations and writes for the same cache key + fc.mu.Lock() + defer fc.mu.Unlock() + + // Try to read from disk cache + cachedData, cacheExists := fc.readFromCacheJSON(ctx, cachePath) + + // Record metrics + if cacheExists { + log.Debugf(ctx, "[Local Cache] cache hit") + fc.addTelemetryMetric("local.cache.hit") + + // If cache is enabled, return the cached value + if fc.cacheEnabled { + return cachedData, nil + } + } else { + log.Debugf(ctx, "[Local Cache] cache miss, computing") + fc.addTelemetryMetric("local.cache.miss") + } + + // Compute the value and measure timing + start := time.Now() + result, err := compute(ctx) + if err != nil { + log.Debugf(ctx, "[Local Cache] error while computing: %v", err) + fc.addTelemetryMetric("local.cache.error") + return result, err + } + + // Record duration metrics + if fc.metrics != nil { + computeDuration := time.Since(start) + fc.metrics.AddDurationValue("local.cache.compute_duration", computeDuration) + } + + log.Debugf(ctx, "[Local Cache] computed and stored result") + + // Write to disk cache (failures are silent - cache write errors don't affect the result) + fc.writeToCacheJSON(ctx, cachePath, result) + + return result, nil +} + +// readFromCacheJSON attempts to read data from the cache file. +// Expiry is checked using file modification time for consistency with cleanup. +func (fc *fileCache) readFromCacheJSON(ctx context.Context, cachePath string) ([]byte, bool) { + // Check file modification time for expiry + info, err := os.Stat(cachePath) + if err != nil { + log.Debugf(ctx, "[Local Cache] failed to stat cache file: %v", err) + return nil, false + } + + if fc.isExpired(info.ModTime()) { + return nil, false + } + + // Read the data + data, err := os.ReadFile(cachePath) + if err != nil { + log.Debugf(ctx, "[Local Cache] failed to read cache file: %v", err) + return nil, false + } + + return data, true +} + +// writeToCacheJSON writes data to the cache file atomically. +// Uses atomic write: writes to temp file first, then renames to actual cache file. +func (fc *fileCache) writeToCacheJSON(ctx context.Context, cachePath string, data []byte) { + // Create temporary file in the same directory for atomic operation + tempFile, err := os.CreateTemp(fc.baseDir, ".cache-*.tmp") + if err != nil { + log.Debugf(ctx, "[Local Cache] failed to create temp cache file: %v", err) + return + } + tempPath := tempFile.Name() + defer func() { + _ = tempFile.Close() + _ = os.Remove(tempPath) // Clean up temp file if still exists + }() + + // Write data to temp file + if _, err := tempFile.Write(data); err != nil { + log.Debugf(ctx, "[Local Cache] failed to write to temp cache file: %v", err) + return + } + + if err := tempFile.Close(); err != nil { + log.Debugf(ctx, "[Local Cache] failed to close temp cache file: %v", err) + return + } + + // On Windows, os.Rename fails if target exists, so remove it first + // This is a best-effort operation - if it fails because file doesn't exist, that's fine + _ = os.Remove(cachePath) + + // Atomically rename temp file to actual cache file + if err := os.Rename(tempPath, cachePath); err != nil { + log.Debugf(ctx, "[Local Cache] failed to rename temp cache file: %v", err) + } +} + +// getCachePath returns the full path to the cache file for a given cache key. +func (fc *fileCache) getCachePath(cacheKey string) string { + return filepath.Join(fc.baseDir, cacheKey+".json") +} diff --git a/libs/cache/file_cache_clear.go b/libs/cache/file_cache_clear.go new file mode 100644 index 00000000000..d4a51647e23 --- /dev/null +++ b/libs/cache/file_cache_clear.go @@ -0,0 +1,34 @@ +package cache + +import ( + "context" + "os" +) + +// ClearFileCache removes all cached files from the Databricks cache directory. +// This clears the cache for ALL CLI versions, not just the current version. +// +// The cache directory structure is: +// +// ~/.cache/databricks/ (or %LOCALAPPDATA%\databricks\ on Windows) +// └── / +// └── / +// ├── .json +// └── ... +// +// This function removes the entire databricks cache directory (all versions and components). +// Returns the path of the cleared directory on success. +func ClearFileCache(ctx context.Context) (string, error) { + databricksCacheDir, err := getCacheBaseDir(ctx) + if err != nil { + return "", err + } + + // Remove the entire databricks cache directory (all versions) + err = os.RemoveAll(databricksCacheDir) + if err != nil { + return "", err + } + + return databricksCacheDir, nil +} diff --git a/libs/cache/file_cache_env_test.go b/libs/cache/file_cache_env_test.go new file mode 100644 index 00000000000..a2e2db4aed7 --- /dev/null +++ b/libs/cache/file_cache_env_test.go @@ -0,0 +1,187 @@ +package cache + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCacheEnabledEnvVar(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + + tests := []struct { + name string + envValue string + expectCached bool + }{ + { + name: "cache enabled with 'true'", + envValue: "true", + expectCached: true, + }, + { + name: "cache disabled with 'false'", + envValue: "false", + expectCached: false, + }, + { + name: "cache disabled when empty", + envValue: "", + expectCached: false, + }, + { + name: "cache disabled with invalid value", + envValue: "yes", + expectCached: false, + }, + { + name: "cache disabled with '1'", + envValue: "1", + expectCached: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a unique subdirectory for this test + testDir := filepath.Join(tempDir, tt.name) + + // Set up context with environment variable + testCtx := ctx + if tt.envValue != "" { + testCtx = env.Set(testCtx, "DATABRICKS_CACHE_ENABLED", tt.envValue) + } + testCtx = env.Set(testCtx, "DATABRICKS_CACHE_DIR", testDir) + + // Use NewCache to properly initialize the cache + cache := NewCache(testCtx, "test-component", 60*time.Minute, nil) + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "test-key", + } + + // First call - should always compute + var computeCalls int32 + result, err := GetOrCompute[string](testCtx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "computed-value", nil + }) + require.NoError(t, err) + assert.Equal(t, "computed-value", result) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls)) + + // Second call - should use cache only if enabled + result2, err := GetOrCompute[string](testCtx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "should-not-be-called", nil + }) + require.NoError(t, err) + + if tt.expectCached { + // Cache enabled - should return cached value + assert.Equal(t, "computed-value", result2) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls), "Should not recompute when cache is enabled") + } else { + // Cache disabled - should recompute + assert.Equal(t, "should-not-be-called", result2) + assert.Equal(t, int32(2), atomic.LoadInt32(&computeCalls), "Should recompute when cache is disabled") + } + }) + } +} + +func TestCacheDirEnvVar(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + + t.Run("uses DATABRICKS_CACHE_DIR when set", func(t *testing.T) { + customCacheDir := filepath.Join(tempDir, "custom-cache") + t.Setenv("DATABRICKS_CACHE_DIR", customCacheDir) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + fc, ok := cache.impl.(*fileCache) + require.True(t, ok) + + // Verify the cache directory is under the custom path + assert.Contains(t, fc.baseDir, customCacheDir) + assert.Contains(t, fc.baseDir, "test-component") + + // Verify directory was created + _, err := os.Stat(customCacheDir) + assert.NoError(t, err, "Custom cache directory should be created") + }) + + t.Run("uses default UserCacheDir when env var not set", func(t *testing.T) { + os.Unsetenv("DATABRICKS_CACHE_DIR") + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + fc, ok := cache.impl.(*fileCache) + require.True(t, ok) + + // Verify it's using the default path structure + userCacheDir, err := os.UserCacheDir() + require.NoError(t, err) + expectedPrefix := filepath.Join(userCacheDir, "databricks") + + assert.Contains(t, fc.baseDir, expectedPrefix) + }) + + t.Run("handles invalid cache dir path", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipping invalid path test on Windows - permission model differs") + } + + // Set an invalid path (no permissions) + t.Setenv("DATABRICKS_CACHE_DIR", "/root/invalid-cache-dir") + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + _, ok := cache.impl.(*noopFileCache) + require.True(t, ok) + }) +} + +func TestCacheIsolationByVersion(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + t.Setenv("DATABRICKS_CACHE_DIR", tempDir) + + // Create cache for component + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + fc, ok := cache.impl.(*fileCache) + require.True(t, ok) + + // Verify the cache path structure: // + // The path should contain the component name + assert.Contains(t, fc.baseDir, "test-component") + + // The path should be a subdirectory of tempDir + assert.Contains(t, fc.baseDir, tempDir) + + // Verify there's at least one intermediate directory between tempDir and component + // (the version directory) + relativePath, err := filepath.Rel(tempDir, fc.baseDir) + require.NoError(t, err) + + // Split by separator and count + pathParts := filepath.SplitList(relativePath) + // On most systems, SplitList is for PATH env var, not file paths + // Use strings.Split instead + if len(pathParts) == 1 { + pathParts = strings.Split(relativePath, string(filepath.Separator)) + } + + // Should have at least 2 parts: / + assert.GreaterOrEqual(t, len(pathParts), 2, "Cache path should include version directory: %s", relativePath) +} diff --git a/libs/cache/file_cache_expiry_test.go b/libs/cache/file_cache_expiry_test.go new file mode 100644 index 00000000000..6ae55888de9 --- /dev/null +++ b/libs/cache/file_cache_expiry_test.go @@ -0,0 +1,98 @@ +package cache + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestFileCacheExpiryBehavior tests that the cache writes files and respects expiry based on mtime +func TestFileCacheExpiryBehavior(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + + // Create cache with 1 minute expiry + fc, err := newFileCacheWithBaseDir(ctx, tempDir, 1*time.Minute) + require.NoError(t, err) + + // Enable cache for this test (default is measurement-only mode) + fc.cacheEnabled = true + + cache := &Cache{impl: fc} + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "test-expiry", + } + + // Compute and store a value + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + return "test-value", nil + }) + require.NoError(t, err) + assert.Equal(t, "test-value", result) + + // Find the cache file and verify it was created + cacheFiles, err := filepath.Glob(filepath.Join(tempDir, "*.json")) + require.NoError(t, err) + require.Len(t, cacheFiles, 1) + + // Verify the file contains the expected data (stored directly, not wrapped) + data, err := os.ReadFile(cacheFiles[0]) + require.NoError(t, err) + assert.Equal(t, `"test-value"`, string(data)) + + // Verify mtime is recent (within last 10 seconds) + info, err := os.Stat(cacheFiles[0]) + require.NoError(t, err) + age := time.Since(info.ModTime()) + assert.Less(t, age, 10*time.Second, "File should have been created recently") + + // Make the file expired by backdating its mtime to 2 minutes ago (older than 1 minute expiry) + expiredTime := time.Now().Add(-2 * time.Minute) + require.NoError(t, os.Chtimes(cacheFiles[0], expiredTime, expiredTime)) + + // Verify GetOrCompute treats it as a cache miss and recomputes + callCount := 0 + result, err = GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + callCount++ + return "recomputed-value", nil + }) + require.NoError(t, err) + assert.Equal(t, "recomputed-value", result, "Should return newly computed value, not expired cache") + assert.Equal(t, 1, callCount, "Should have called compute function once due to cache expiry") +} + +// TestReadFromCacheRespectsExpiry tests that readFromCacheJSON returns false for expired entries based on mtime +func TestReadFromCacheRespectsExpiry(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + cache, err := newFileCacheWithBaseDir(ctx, tempDir, 1*time.Minute) // 1 minute expiry + require.NoError(t, err) + + // Create an expired cache file by setting its mtime to 2 hours ago + expiredFile := filepath.Join(tempDir, "expired.json") + require.NoError(t, os.WriteFile(expiredFile, []byte(`"expired-value"`), 0o644)) + oldTime := time.Now().Add(-2 * time.Hour) + require.NoError(t, os.Chtimes(expiredFile, oldTime, oldTime)) + + // Try to read from expired cache - should return false + result, found := cache.readFromCacheJSON(ctx, expiredFile) + assert.False(t, found, "Should not find expired cache entry") + assert.Nil(t, result, "Result should be nil for expired entry") + + // Create a valid (non-expired) cache file with recent mtime + validFile := filepath.Join(tempDir, "valid.json") + require.NoError(t, os.WriteFile(validFile, []byte(`"valid-value"`), 0o644)) + + // Try to read from valid cache - should return true + result, found = cache.readFromCacheJSON(ctx, validFile) + assert.True(t, found, "Should find valid cache entry") + assert.Equal(t, `"valid-value"`, string(result), "Should return correct value for valid entry") +} diff --git a/libs/cache/file_cache_test.go b/libs/cache/file_cache_test.go new file mode 100644 index 00000000000..214c400e7fe --- /dev/null +++ b/libs/cache/file_cache_test.go @@ -0,0 +1,433 @@ +package cache + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewFileCache(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + cacheDir := filepath.Join(tempDir, "cache") + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "true") + ctx = env.Set(ctx, "DATABRICKS_CACHE_DIR", cacheDir) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + fc, ok := cache.impl.(*fileCache) + require.True(t, ok) + assert.True(t, strings.HasPrefix(fc.baseDir, cacheDir)) + + // Verify directory was created + info, err := os.Stat(cacheDir) + require.NoError(t, err) + assert.True(t, info.IsDir()) + + // Check permissions - Windows has different permission semantics + if runtime.GOOS != "windows" { + assert.Equal(t, os.FileMode(0o700), info.Mode().Perm()) + } else { + // On Windows, verify directory is accessible by trying to create a test file + testFile := filepath.Join(cacheDir, "test_access") + err := os.WriteFile(testFile, []byte("test"), 0o600) + assert.NoError(t, err) + if err == nil { + _ = os.Remove(testFile) + } + } +} + +func TestNewFileCacheWithExistingDirectory(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + cacheDir := filepath.Join(tempDir, "existing") + + // Create directory first + err := os.MkdirAll(cacheDir, 0o700) + require.NoError(t, err) + + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "true") + ctx = env.Set(ctx, "DATABRICKS_CACHE_DIR", cacheDir) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + fc, ok := cache.impl.(*fileCache) + require.True(t, ok) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(fc.baseDir, cacheDir)) +} + +func TestNewFileCacheInvalidPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipping invalid path test on Windows") + } + + ctx := context.Background() + // Try to create cache in a location that should fail + invalidPath := "/root/invalid/path/that/should/not/exist" + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "true") + ctx = env.Set(ctx, "DATABRICKS_CACHE_DIR", invalidPath) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + _, ok := cache.impl.(*noopFileCache) + require.True(t, ok) +} + +func TestFileCacheGetOrCompute(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + cacheDir := filepath.Join(tempDir, "cache") + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "true") + ctx = env.Set(ctx, "DATABRICKS_CACHE_DIR", cacheDir) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + + fingerprint := struct { + Key string `json:"key"` + Value int `json:"value"` + }{ + Key: "test-key", + Value: 123, + } + expectedValue := "computed-value" + + // First call should compute the value + var computeCalls int32 + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return expectedValue, nil + }) + + require.NoError(t, err) + assert.Equal(t, expectedValue, result) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls)) + + // Second call should return cached value without computing + result2, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "should-not-be-called", nil + }) + + require.NoError(t, err) + assert.Equal(t, expectedValue, result2) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls)) +} + +func TestFileCacheGetOrComputeError(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + cacheDir := filepath.Join(tempDir, "cache") + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "true") + ctx = env.Set(ctx, "DATABRICKS_CACHE_DIR", cacheDir) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "error-key", + } + + // Compute function returns error + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + return "", assert.AnError + }) + + assert.Empty(t, result) + assert.Error(t, err) + assert.Equal(t, assert.AnError, err) +} + +func TestFileCacheGetOrComputeConcurrency(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + cacheDir := filepath.Join(tempDir, "cache") + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "true") + ctx = env.Set(ctx, "DATABRICKS_CACHE_DIR", cacheDir) + + cache := NewCache(ctx, "test-component", 60*time.Minute, nil) + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "concurrent-key", + } + expectedValue := "concurrent-value" + var computeCalls int32 + + // Start multiple goroutines that try to compute the same key + numGoroutines := 10 + results := make(chan any, numGoroutines) + errors := make(chan error, numGoroutines) + + for range numGoroutines { + go func() { + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + time.Sleep(10 * time.Millisecond) // Simulate work + return expectedValue, nil + }) + results <- result + errors <- err + }() + } + + // Collect all results + for range numGoroutines { + result := <-results + err := <-errors + require.NoError(t, err) + assert.Equal(t, expectedValue, result) + } + + // With locking, writes are serialized but compute may be called multiple times + // since goroutines check cache before acquiring lock + calls := atomic.LoadInt32(&computeCalls) + assert.GreaterOrEqual(t, calls, int32(1), "compute should be called at least once") + assert.LessOrEqual(t, calls, int32(numGoroutines), "compute should not be called more than number of goroutines") +} + +func TestFileCacheCleanupExpiredFiles(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + expiry := 60 * time.Minute + + // Create some cache files manually - one expired, one valid + now := time.Now() + + // Expired file - create it and set mtime to make it appear old + expiredFile := filepath.Join(tempDir, "expired.json") + require.NoError(t, os.WriteFile(expiredFile, []byte(`"expired-value"`), 0o644)) + // Set mtime to 2 hours ago (older than expiry) + oldTime := now.Add(-2 * time.Hour) + require.NoError(t, os.Chtimes(expiredFile, oldTime, oldTime)) + + // Valid file - recently created + validFile := filepath.Join(tempDir, "valid.json") + require.NoError(t, os.WriteFile(validFile, []byte(`"valid-value"`), 0o644)) + + // Non-cache file (should be ignored) + nonCacheFile := filepath.Join(tempDir, "readme.txt") + require.NoError(t, os.WriteFile(nonCacheFile, []byte("readme"), 0o644)) + + // Create cache - this should trigger cleanup + _, err := newFileCacheWithBaseDir(ctx, tempDir, expiry) + require.NoError(t, err) + + // Check results + _, err = os.Stat(expiredFile) + assert.True(t, os.IsNotExist(err), "Expired file should be deleted") + + _, err = os.Stat(validFile) + assert.False(t, os.IsNotExist(err), "Valid file should still exist") + + _, err = os.Stat(nonCacheFile) + assert.False(t, os.IsNotExist(err), "Non-cache file should be ignored") +} + +func TestFileCacheInvalidJSON(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + fc, err := newFileCacheWithBaseDir(ctx, tempDir, 60*time.Minute) + require.NoError(t, err) + + // Enable cache for this test + fc.cacheEnabled = true + + cache := &Cache{impl: fc} + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "test-invalid-json", + } + + // Manually write invalid JSON to the cache file + cacheKey, err := fingerprintToHash(fingerprint) + require.NoError(t, err) + cachePath := fc.getCachePath(cacheKey) + err = os.WriteFile(cachePath, []byte("invalid json {{{"), 0o600) + require.NoError(t, err) + + // GetOrCompute should fail open and recompute when cache contains invalid JSON + var computeCalls int32 + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "recomputed-value", nil + }) + + require.NoError(t, err) + assert.Equal(t, "recomputed-value", result) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls), "Should recompute when cache has invalid JSON") +} + +func TestFileCacheCorruptedData(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + fc, err := newFileCacheWithBaseDir(ctx, tempDir, 60*time.Minute) + require.NoError(t, err) + + // Enable cache for this test + fc.cacheEnabled = true + + cache := &Cache{impl: fc} + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "test-corrupted", + } + + // Write valid JSON but wrong type (string instead of int) + cacheKey, err := fingerprintToHash(fingerprint) + require.NoError(t, err) + cachePath := fc.getCachePath(cacheKey) + err = os.WriteFile(cachePath, []byte(`"not-an-integer"`), 0o600) + require.NoError(t, err) + + // GetOrCompute should fail open and recompute when cache type doesn't match + var computeCalls int32 + result, err := GetOrCompute[int](ctx, cache, fingerprint, func(ctx context.Context) (int, error) { + atomic.AddInt32(&computeCalls, 1) + return 42, nil + }) + + require.NoError(t, err) + assert.Equal(t, 42, result) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls), "Should recompute when cache type is wrong") +} + +func TestFileCacheEmptyFingerprint(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + fc, err := newFileCacheWithBaseDir(ctx, tempDir, 60*time.Minute) + require.NoError(t, err) + + // Enable cache for this test + fc.cacheEnabled = true + + cache := &Cache{impl: fc} + + // Empty struct fingerprint is valid + fingerprint := struct{}{} + + var computeCalls int32 + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "value", nil + }) + require.NoError(t, err) + assert.Equal(t, "value", result) + + // Second call should use cache + result2, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "should-not-be-called", nil + }) + require.NoError(t, err) + assert.Equal(t, "value", result2) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls), "Empty fingerprint should work with cache") +} + +func TestFileCacheMeasurementMode(t *testing.T) { + ctx := context.Background() + tempDir := t.TempDir() + fc, err := newFileCacheWithBaseDir(ctx, tempDir, 60*time.Minute) + require.NoError(t, err) + + // Keep cache disabled (measurement mode) + fc.cacheEnabled = false + + cache := &Cache{impl: fc} + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "test-measurement", + } + + // First call + var computeCalls int32 + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "computed-value", nil + }) + require.NoError(t, err) + assert.Equal(t, "computed-value", result) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls)) + + // Second call - in measurement mode, should always recompute + result2, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "recomputed-value", nil + }) + require.NoError(t, err) + assert.Equal(t, "recomputed-value", result2) + assert.Equal(t, int32(2), atomic.LoadInt32(&computeCalls), "Measurement mode should always recompute") + + // But cache file should still exist + cacheFiles, err := filepath.Glob(filepath.Join(tempDir, "*.json")) + require.NoError(t, err) + assert.Len(t, cacheFiles, 1, "Cache file should be written even in measurement mode") +} + +func TestFileCacheReadPermissionError(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("Skipping permission test when running as root") + } + if runtime.GOOS == "windows" { + t.Skip("Skipping permission test on Windows") + } + + ctx := context.Background() + tempDir := t.TempDir() + fc, err := newFileCacheWithBaseDir(ctx, tempDir, 60*time.Minute) + require.NoError(t, err) + + // Enable cache for this test + fc.cacheEnabled = true + + cache := &Cache{impl: fc} + + fingerprint := struct { + Key string `json:"key"` + }{ + Key: "test-permissions", + } + + // First, populate the cache + result, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + return "cached-value", nil + }) + require.NoError(t, err) + assert.Equal(t, "cached-value", result) + + // Find the cache file and make it unreadable + cacheFiles, err := filepath.Glob(filepath.Join(tempDir, "*.json")) + require.NoError(t, err) + require.Len(t, cacheFiles, 1) + err = os.Chmod(cacheFiles[0], 0o000) + require.NoError(t, err) + + // Restore permissions after test + defer func() { _ = os.Chmod(cacheFiles[0], 0o600) }() + + // GetOrCompute should fail open and recompute when file is unreadable + var computeCalls int32 + result2, err := GetOrCompute[string](ctx, cache, fingerprint, func(ctx context.Context) (string, error) { + atomic.AddInt32(&computeCalls, 1) + return "recomputed-value", nil + }) + + require.NoError(t, err) + assert.Equal(t, "recomputed-value", result2) + assert.Equal(t, int32(1), atomic.LoadInt32(&computeCalls), "Should recompute when cache file is unreadable") +} diff --git a/libs/cache/fingerprint.go b/libs/cache/fingerprint.go new file mode 100644 index 00000000000..8f866405122 --- /dev/null +++ b/libs/cache/fingerprint.go @@ -0,0 +1,22 @@ +package cache + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" +) + +// fingerprintToHash converts any struct to a deterministic string representation for use as a cache key. +func fingerprintToHash(fingerprint any) (string, error) { + // Marshal map - json.Marshal sorts map keys alphabetically + data, err := json.Marshal(fingerprint) + if err != nil { + return "", fmt.Errorf("failed to marshal normalized fingerprint: %w", err) + } + + // Hash for consistent, reasonably-sized key. + // hash[:] converts the [32]byte array returned by Sum256 to a []byte slice. + hash := sha256.Sum256(data) + return hex.EncodeToString(hash[:]), nil +} diff --git a/libs/cache/fingerprint_test.go b/libs/cache/fingerprint_test.go new file mode 100644 index 00000000000..8a85f7944ce --- /dev/null +++ b/libs/cache/fingerprint_test.go @@ -0,0 +1,34 @@ +package cache + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestFingerprintStability tests that the fingerprintToHash function returns the same hash for the same input. +func TestFingerprintStability(t *testing.T) { + fingerprint1 := struct { + Key string `json:"key"` + }{ + Key: "test-key", + } + + fingerprint2 := struct { + Key string `json:"key"` + }{ + Key: "test-key2", + } + + hash1, err := fingerprintToHash(fingerprint1) + require.NoError(t, err) + require.Equal(t, "1b329dc07a9fa87da7480f6b10cc917a40a4f460ac82aea3d09df477764f3101", hash1) + hash2, err := fingerprintToHash(fingerprint2) + require.NoError(t, err) + hash1ToCompare, err := fingerprintToHash(fingerprint1) + require.NoError(t, err) + + assert.Equal(t, hash1ToCompare, hash1) + assert.NotEqual(t, hash1, hash2) +} diff --git a/libs/cache/noop_file_cache.go b/libs/cache/noop_file_cache.go new file mode 100644 index 00000000000..4b71be43fc0 --- /dev/null +++ b/libs/cache/noop_file_cache.go @@ -0,0 +1,9 @@ +package cache + +import "context" + +type noopFileCache struct{} + +func (c *noopFileCache) getOrComputeJSON(ctx context.Context, fingerprint any, compute func(ctx context.Context) ([]byte, error)) ([]byte, error) { + return compute(ctx) +} diff --git a/libs/telemetry/protos/bundle_deploy.go b/libs/telemetry/protos/bundle_deploy.go index b7e3075811a..ab1b3a46de5 100644 --- a/libs/telemetry/protos/bundle_deploy.go +++ b/libs/telemetry/protos/bundle_deploy.go @@ -80,6 +80,9 @@ type BundleDeployExperimental struct { // Number of resource mutators declared at 'python/mutators' in databricks.yml PythonResourceMutatorsCount int64 `json:"python_resource_mutators_count,omitempty"` + + // Local cache measurements in milliseconds (compute duration, potential savings, etc.) + LocalCacheMeasurementsMs []IntMapEntry `json:"local_cache_measurements_ms,omitempty"` } type BoolMapEntry struct { @@ -89,5 +92,5 @@ type BoolMapEntry struct { type IntMapEntry struct { Key string `json:"key,omitempty"` - Value int64 `json:"value,omitempty"` + Value int64 `json:"value"` }