Skip to content

Commit 47e11ad

Browse files
committed
deployCore: use Finalize return value instead of re-opening state
Finalize now returns (ExportedResourcesMap, error), capturing the merged state before clearing. This lets deploy.go use LoadFromState directly instead of closing and re-opening the state file from disk. LoadFromState is a new statemgmt constructor that accepts pre-computed state and skips the engine dispatch in Load.Apply. Co-authored-by: Denis Bilenko <denis.bilenko@databricks.com>
1 parent 4ef7c16 commit 47e11ad

8 files changed

Lines changed: 100 additions & 50 deletions

File tree

bundle/direct/bind.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
6464
var checkStateDB dstate.DeploymentState
6565
if err := checkStateDB.Open(ctx, statePath, dstate.WithRecovery(true), dstate.WithWrite(false)); err == nil {
6666
existingID := checkStateDB.GetResourceID(resourceKey)
67-
_ = checkStateDB.Finalize(ctx)
67+
if _, err := checkStateDB.Finalize(ctx); err != nil {
68+
log.Warnf(ctx, "failed to finalize state: %v", err)
69+
}
6870
if existingID != "" {
6971
return nil, ErrResourceAlreadyBound{
7072
ResourceKey: resourceKey,
@@ -98,7 +100,7 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
98100
}
99101

100102
// Finalize to persist temp state to disk
101-
err = b.StateDB.Finalize(ctx)
103+
_, err = b.StateDB.Finalize(ctx)
102104
if err != nil {
103105
os.Remove(tmpStatePath)
104106
return nil, err
@@ -117,7 +119,9 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
117119
os.Remove(tmpStatePath)
118120
return nil, err
119121
}
120-
_ = b.StateDB.Finalize(ctx)
122+
if _, err := b.StateDB.Finalize(ctx); err != nil {
123+
log.Warnf(ctx, "failed to finalize state: %v", err)
124+
}
121125

122126
// Populate the state with the resolved config
123127
entry := plan.Plan[resourceKey]
@@ -152,7 +156,7 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
152156
return nil, err
153157
}
154158

155-
err = b.StateDB.Finalize(ctx)
159+
_, err = b.StateDB.Finalize(ctx)
156160
if err != nil {
157161
os.Remove(tmpStatePath)
158162
return nil, err
@@ -166,7 +170,9 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
166170
return nil, err
167171
}
168172
plan, err = b.CalculatePlan(ctx, client, configRoot)
169-
_ = b.StateDB.Finalize(ctx)
173+
if _, ferr := b.StateDB.Finalize(ctx); ferr != nil {
174+
log.Warnf(ctx, "failed to finalize state: %v", ferr)
175+
}
170176
if err != nil {
171177
os.Remove(tmpStatePath)
172178
return nil, err
@@ -236,5 +242,6 @@ func (b *DeploymentBundle) Unbind(ctx context.Context, statePath, resourceKey st
236242
}
237243
}
238244

239-
return b.StateDB.Finalize(ctx)
245+
_, err = b.StateDB.Finalize(ctx)
246+
return err
240247
}

bundle/direct/dstate/state.go

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,25 @@ const (
3232
var errStaleWAL = errors.New("stale WAL")
3333

3434
type DeploymentState struct {
35-
Path string
36-
Data Database
37-
mu sync.Mutex
38-
walFile *os.File
35+
Path string
36+
Data Database
37+
mu sync.Mutex
38+
walFile *os.File
39+
40+
// Maps resource key to ID. Unlike Data.State, this is up to during writes (deploys).
3941
stateIDs map[string]string
4042
}
4143

4244
type Database struct {
43-
StateVersion int `json:"state_version"`
44-
CLIVersion string `json:"cli_version"`
45-
Lineage string `json:"lineage"`
46-
Serial int `json:"serial"`
47-
State map[string]ResourceEntry `json:"state"`
45+
StateVersion int `json:"state_version"`
46+
CLIVersion string `json:"cli_version"`
47+
Lineage string `json:"lineage"`
48+
Serial int `json:"serial"`
49+
50+
// Maps resource key to ResourceEntry which includes ID + full serialized state.
51+
// This is not updated during write/deploy, those writes go to WAL instead.
52+
// The State is then reconstructed from WAL.
53+
State map[string]ResourceEntry `json:"state"`
4854
}
4955

5056
type ResourceEntry struct {
@@ -346,14 +352,15 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error)
346352
return lineNumber > 1, nil
347353
}
348354

349-
// Finalize replays the WAL (if open for write) and resets the state.
355+
// Finalize replays the WAL (if open for write), captures the resulting state, and resets.
350356
// Safe to call multiple times or on an already-finalized state.
351-
func (db *DeploymentState) Finalize(ctx context.Context) error {
357+
// Returns the exported state as of the end of this operation.
358+
func (db *DeploymentState) Finalize(ctx context.Context) (resourcestate.ExportedResourcesMap, error) {
352359
db.mu.Lock()
353360
defer db.mu.Unlock()
354361

355362
if db.Path == "" {
356-
return nil
363+
return nil, nil
357364
}
358365

359366
var err error
@@ -364,11 +371,13 @@ func (db *DeploymentState) Finalize(ctx context.Context) error {
364371
err = db.replayWAL(ctx)
365372
}
366373

374+
state := ExportStateFromData(db.Data)
375+
367376
db.Path = ""
368377
db.Data = Database{}
369-
db.stateIDs = make(map[string]string)
378+
db.stateIDs = nil
370379

371-
return err
380+
return state, err
372381
}
373382

374383
// UpgradeToWrite transitions from read mode to write mode without re-reading state.
@@ -427,9 +436,10 @@ func (db *DeploymentState) AssertOpenedForWrite() {
427436
}
428437
}
429438

430-
func (db *DeploymentState) ExportState(ctx context.Context) resourcestate.ExportedResourcesMap {
439+
// ExportStateFromData extracts resource IDs and ETags from a database snapshot.
440+
func ExportStateFromData(data Database) resourcestate.ExportedResourcesMap {
431441
result := make(resourcestate.ExportedResourcesMap)
432-
for key, entry := range db.Data.State {
442+
for key, entry := range data.State {
433443
var etag string
434444
// Extract etag for dashboards.
435445
// covered by test case: bundle/deploy/dashboard/detect-change
@@ -450,6 +460,10 @@ func (db *DeploymentState) ExportState(ctx context.Context) resourcestate.Export
450460
return result
451461
}
452462

463+
func (db *DeploymentState) ExportState(ctx context.Context) resourcestate.ExportedResourcesMap {
464+
return ExportStateFromData(db.Data)
465+
}
466+
453467
func (db *DeploymentState) unlockedSave() error {
454468
data, err := json.MarshalIndent(db.Data, "", " ")
455469
if err != nil {

bundle/direct/dstate/state_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,35 @@ import (
99
"github.com/stretchr/testify/require"
1010
)
1111

12+
func mustFinalize(t *testing.T, db *DeploymentState) {
13+
t.Helper()
14+
_, err := db.Finalize(t.Context())
15+
require.NoError(t, err)
16+
}
17+
1218
func TestOpenSaveFinalizeRoundTrip(t *testing.T) {
1319
path := filepath.Join(t.TempDir(), "state.json")
1420

1521
var db DeploymentState
1622
require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
1723

1824
require.NoError(t, db.SaveState("jobs.my_job", "123", map[string]string{"key": "val"}, nil))
19-
require.NoError(t, db.Finalize(t.Context()))
25+
mustFinalize(t, &db)
2026

2127
// Re-open and verify persisted data.
2228
var db2 DeploymentState
2329
require.NoError(t, db2.Open(t.Context(), path, WithRecovery(false), WithWrite(false)))
2430
assert.Equal(t, 1, db2.Data.Serial)
2531
assert.Equal(t, "123", db2.GetResourceID("jobs.my_job"))
26-
require.NoError(t, db2.Finalize(t.Context()))
32+
mustFinalize(t, &db2)
2733
}
2834

2935
func TestFinalizeWithNoEntriesDoesNotWriteStateFile(t *testing.T) {
3036
path := filepath.Join(t.TempDir(), "state.json")
3137

3238
var db DeploymentState
3339
require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
34-
require.NoError(t, db.Finalize(t.Context()))
40+
mustFinalize(t, &db)
3541

3642
_, err := os.Stat(path)
3743
assert.ErrorIs(t, err, os.ErrNotExist)
@@ -46,7 +52,7 @@ func TestPanicOnDoubleOpen(t *testing.T) {
4652
assert.Panics(t, func() {
4753
_ = db.Open(t.Context(), path, WithRecovery(true), WithWrite(true))
4854
})
49-
require.NoError(t, db.Finalize(t.Context()))
55+
mustFinalize(t, &db)
5056
}
5157

5258
func TestDeleteState(t *testing.T) {
@@ -55,16 +61,16 @@ func TestDeleteState(t *testing.T) {
5561
var db DeploymentState
5662
require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
5763
require.NoError(t, db.SaveState("jobs.my_job", "123", map[string]string{}, nil))
58-
require.NoError(t, db.Finalize(t.Context()))
64+
mustFinalize(t, &db)
5965

6066
var db2 DeploymentState
6167
require.NoError(t, db2.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
6268
require.NoError(t, db2.DeleteState("jobs.my_job"))
63-
require.NoError(t, db2.Finalize(t.Context()))
69+
mustFinalize(t, &db2)
6470

6571
var db3 DeploymentState
6672
require.NoError(t, db3.Open(t.Context(), path, WithRecovery(false), WithWrite(false)))
6773
assert.Equal(t, 2, db3.Data.Serial)
6874
assert.Equal(t, "", db3.GetResourceID("jobs.my_job"))
69-
require.NoError(t, db3.Finalize(t.Context()))
75+
mustFinalize(t, &db3)
7076
}

bundle/phases/deploy.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/databricks/cli/bundle/deploy/terraform"
1616
"github.com/databricks/cli/bundle/deployplan"
1717
"github.com/databricks/cli/bundle/direct"
18-
"github.com/databricks/cli/bundle/direct/dstate"
1918
"github.com/databricks/cli/bundle/libraries"
2019
"github.com/databricks/cli/bundle/metrics"
2120
"github.com/databricks/cli/bundle/permissions"
@@ -75,14 +74,12 @@ func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, ta
7574
bundle.ApplyContext(ctx, b, terraform.Apply())
7675
}
7776

78-
// Close state to replay WAL into state file, then reopen for read.
79-
// PushResourcesState needs the file on disk, Load needs the state in memory.
77+
// Flush WAL to state file on disk; capture the resulting state for Load below.
78+
var directState statemgmt.ExportedResourcesMap
8079
if targetEngine.IsDirect() {
81-
if err := b.DeploymentBundle.StateDB.Finalize(ctx); err != nil {
82-
logdiag.LogError(ctx, err)
83-
}
84-
_, localPath := b.StateFilenameDirect(ctx)
85-
if err := b.DeploymentBundle.StateDB.Open(ctx, localPath, dstate.WithRecovery(true), dstate.WithWrite(false)); err != nil {
80+
var err error
81+
directState, err = b.DeploymentBundle.StateDB.Finalize(ctx)
82+
if err != nil {
8683
logdiag.LogError(ctx, err)
8784
}
8885
}
@@ -93,8 +90,15 @@ func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, ta
9390
return
9491
}
9592

93+
var loadMutator bundle.Mutator
94+
if targetEngine.IsDirect() {
95+
loadMutator = statemgmt.LoadFromState(directState)
96+
} else {
97+
loadMutator = statemgmt.Load(targetEngine)
98+
}
99+
96100
bundle.ApplySeqContext(ctx, b,
97-
statemgmt.Load(targetEngine),
101+
loadMutator,
98102
metadata.Compute(),
99103
metadata.Upload(),
100104
statemgmt.UploadStateForYamlSync(targetEngine),

bundle/phases/destroy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, e
8686
// Warn instead of hard-error: resources are already deleted, so proceed
8787
// with file cleanup regardless of whether state flush succeeds.
8888
if engine.IsDirect() {
89-
if err := b.DeploymentBundle.StateDB.Finalize(ctx); err != nil {
89+
if _, err := b.DeploymentBundle.StateDB.Finalize(ctx); err != nil {
9090
diags := diag.WarningFromErr(err)
9191
if len(diags) > 0 {
9292
logdiag.LogDiag(ctx, diags[0])

bundle/statemgmt/state_load.go

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ func (l *load) Name() string {
3535
}
3636

3737
func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
38-
var err error
3938
var state ExportedResourcesMap
4039

4140
if l.engine.IsDirect() {
@@ -48,14 +47,29 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
4847
}
4948
}
5049

51-
err = l.validateState(state)
52-
if err != nil {
50+
return applyState(ctx, b, state, l.modes)
51+
}
52+
53+
type loadFromState struct {
54+
state ExportedResourcesMap
55+
modes []LoadMode
56+
}
57+
58+
func (l *loadFromState) Name() string {
59+
return "statemgmt.Load"
60+
}
61+
62+
func (l *loadFromState) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
63+
return applyState(ctx, b, l.state, l.modes)
64+
}
65+
66+
// applyState merges the exported resource state into the bundle configuration.
67+
func applyState(ctx context.Context, b *bundle.Bundle, state ExportedResourcesMap, modes []LoadMode) diag.Diagnostics {
68+
if err := validateLoadedState(state, modes); err != nil {
5369
return diag.FromErr(err)
5470
}
5571

56-
// Merge state into configuration.
57-
err = StateToBundle(ctx, state, &b.Config)
58-
if err != nil {
72+
if err := StateToBundle(ctx, state, &b.Config); err != nil {
5973
return diag.FromErr(err)
6074
}
6175

@@ -160,14 +174,19 @@ func StateToBundle(ctx context.Context, state ExportedResourcesMap, config *conf
160174
})
161175
}
162176

163-
func (l *load) validateState(state ExportedResourcesMap) error {
164-
if len(state) == 0 && slices.Contains(l.modes, ErrorOnEmptyState) {
177+
func validateLoadedState(state ExportedResourcesMap, modes []LoadMode) error {
178+
if len(state) == 0 && slices.Contains(modes, ErrorOnEmptyState) {
165179
return errors.New("resource not found or not yet deployed. Did you forget to run 'databricks bundle deploy'?")
166180
}
167-
168181
return nil
169182
}
170183

171184
func Load(engine engine.EngineType, modes ...LoadMode) bundle.Mutator {
172185
return &load{modes: modes, engine: engine}
173186
}
187+
188+
// LoadFromState returns a mutator that loads the provided pre-computed state into the bundle,
189+
// skipping the engine-specific state retrieval step.
190+
func LoadFromState(state ExportedResourcesMap, modes ...LoadMode) bundle.Mutator {
191+
return &loadFromState{state: state, modes: modes}
192+
}

bundle/statemgmt/upload_state_for_yaml_sync.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func (m *uploadStateForYamlSync) convertState(ctx context.Context, b *bundle.Bun
198198
}
199199

200200
deploymentBundle.Apply(ctx, b.WorkspaceClient(ctx), plan, direct.MigrateMode(true))
201-
if err := deploymentBundle.StateDB.Finalize(ctx); err != nil {
201+
if _, err := deploymentBundle.StateDB.Finalize(ctx); err != nil {
202202
return false, err
203203
}
204204

cmd/bundle/deployment/migrate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ To start using direct engine, set "engine: direct" under bundle in your databric
282282
}
283283

284284
deploymentBundle.Apply(ctx, b.WorkspaceClient(ctx), plan, direct.MigrateMode(true))
285-
if err := deploymentBundle.StateDB.Finalize(ctx); err != nil {
285+
if _, err := deploymentBundle.StateDB.Finalize(ctx); err != nil {
286286
logdiag.LogError(ctx, err)
287287
}
288288
if logdiag.HasError(ctx) {

0 commit comments

Comments
 (0)