From ae14c64a6fae6027556cf110d7f7bf18738b92ef Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 8 Aug 2022 16:06:03 +0100 Subject: [PATCH 1/7] indexer cleanup: Replace Defer with DependsOn --- internal/indexer/walker.go | 48 ++++++------------- internal/indexer/watcher.go | 61 +++++++++---------------- internal/terraform/module/module_ops.go | 13 +++++- 3 files changed, 47 insertions(+), 75 deletions(-) diff --git a/internal/indexer/walker.go b/internal/indexer/walker.go index b803a37e6..9eac6e323 100644 --- a/internal/indexer/walker.go +++ b/internal/indexer/walker.go @@ -111,47 +111,29 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand } if dataDir.PluginLockFilePath != "" { - pSchemaId, err := idx.jobStore.EnqueueJob(job.Job{ + pSchemaVerId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { return module.ParseProviderVersions(idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseProviderVersions.String(), DependsOn: providerVersionDeps, - Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { - ids := make(job.IDs, 0) - - pReqs, err := idx.modStore.ProviderRequirementsForModule(modHandle.Path()) - if err != nil { - return ids, err - } - - exist, err := idx.schemaStore.AllSchemasExist(pReqs) - if err != nil { - return ids, err - } - if exist { - idx.logger.Printf("Avoiding obtaining schemas as they all exist: %#v", pReqs) - // avoid obtaining schemas if we already have it - return ids, nil - } - idx.logger.Printf("Obtaining schemas for: %#v", pReqs) - - id, err := idx.jobStore.EnqueueJob(job.Job{ - Dir: modHandle, - Func: func(ctx context.Context) error { - ctx = exec.WithExecutorFactory(ctx, idx.tfExecFactory) - return module.ObtainSchema(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) - }, - Type: op.OpTypeObtainSchema.String(), - }) - if err != nil { - return ids, err - } - ids = append(ids, id) + }) + if err != nil { + errs = multierror.Append(errs, err) + } else { + ids = append(ids, pSchemaVerId) + refCollectionDeps = append(refCollectionDeps, pSchemaVerId) + } - return ids, nil + pSchemaId, err := idx.jobStore.EnqueueJob(job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + ctx = exec.WithExecutorFactory(ctx, idx.tfExecFactory) + return module.ObtainSchema(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, + Type: op.OpTypeObtainSchema.String(), + DependsOn: job.IDs{pSchemaVerId}, }) if err != nil { errs = multierror.Append(errs, err) diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index 6fa667af4..8764385bc 100644 --- a/internal/indexer/watcher.go +++ b/internal/indexer/watcher.go @@ -3,6 +3,7 @@ package indexer import ( "context" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/terraform/exec" @@ -33,55 +34,35 @@ func (idx *Indexer) ModuleManifestChanged(ctx context.Context, modHandle documen func (idx *Indexer) PluginLockChanged(ctx context.Context, modHandle document.DirHandle) (job.IDs, error) { ids := make(job.IDs, 0) + var errs *multierror.Error - id, err := idx.jobStore.EnqueueJob(job.Job{ + pSchemaVerId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { return module.ParseProviderVersions(idx.fs, idx.modStore, modHandle.Path()) }, - Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { - ids := make(job.IDs, 0) - - mod, err := idx.modStore.ModuleByPath(modHandle.Path()) - if err != nil { - return ids, err - } - - exist, err := idx.schemaStore.AllSchemasExist(mod.Meta.ProviderRequirements) - if err != nil { - return ids, err - } - if exist { - // avoid obtaining schemas if we already have it - return ids, nil - } - - id, err := idx.jobStore.EnqueueJob(job.Job{ - Dir: modHandle, - Func: func(ctx context.Context) error { - ctx = exec.WithExecutorFactory(ctx, idx.tfExecFactory) - eo, ok := exec.ExecutorOptsFromContext(ctx) - if ok { - ctx = exec.WithExecutorOpts(ctx, eo) - } - - return module.ObtainSchema(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) - }, - Type: op.OpTypeObtainSchema.String(), - }) - if err != nil { - return ids, err - } - ids = append(ids, id) + Type: op.OpTypeParseProviderVersions.String(), + }) + if err != nil { + errs = multierror.Append(errs, err) + } else { + ids = append(ids, pSchemaVerId) + } - return ids, nil + pSchemaId, err := idx.jobStore.EnqueueJob(job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + ctx = exec.WithExecutorFactory(ctx, idx.tfExecFactory) + return module.ObtainSchema(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeParseProviderVersions.String(), + Type: op.OpTypeObtainSchema.String(), + DependsOn: job.IDs{pSchemaVerId}, }) if err != nil { - return ids, err + errs = multierror.Append(errs, err) + } else { + ids = append(ids, pSchemaId) } - ids = append(ids, id) - return ids, nil + return ids, errs.ErrorOrNil() } diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 8b672eab0..e0f7e1ebe 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -103,12 +103,21 @@ func providerVersionsFromTfVersion(pv map[string]*version.Version) map[tfaddr.Pr } func ObtainSchema(ctx context.Context, modStore *state.ModuleStore, schemaStore *state.ProviderSchemaStore, modPath string) error { - mod, err := modStore.ModuleByPath(modPath) + pReqs, err := modStore.ProviderRequirementsForModule(modPath) if err != nil { return err } - tfExec, err := TerraformExecutorForModule(ctx, mod.Path) + exist, err := schemaStore.AllSchemasExist(pReqs) + if err != nil { + return err + } + if exist { + // avoid obtaining schemas if we already have it + return nil + } + + tfExec, err := TerraformExecutorForModule(ctx, modPath) if err != nil { sErr := modStore.FinishProviderSchemaLoading(modPath, err) if sErr != nil { From e71a213d3fc7c45186768d109afd27aca5ad4397 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 19 Jul 2022 09:00:26 +0100 Subject: [PATCH 2/7] Introduce StateIgnore & plumb context through --- internal/indexer/document_change.go | 6 +- internal/indexer/document_open.go | 4 +- internal/indexer/module_calls.go | 6 +- internal/indexer/walker.go | 8 +- internal/indexer/watcher.go | 2 +- internal/job/ignore_state.go | 30 ++++++ internal/job/job.go | 18 ++-- internal/scheduler/scheduler.go | 2 + internal/scheduler/scheduler_test.go | 61 +++++++++++ internal/state/jobs.go | 4 +- internal/terraform/module/module_ops.go | 106 ++++++++++++++++--- internal/terraform/module/module_ops_test.go | 12 +-- 12 files changed, 220 insertions(+), 39 deletions(-) create mode 100644 internal/job/ignore_state.go diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index b0840fb08..84c237f2e 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -15,7 +15,7 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro parseId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseModuleConfiguration(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseModuleConfiguration.String(), }) @@ -33,7 +33,7 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro parseVarsId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseVariables(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseVariables.String(), }) @@ -64,7 +64,7 @@ func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs metaId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.LoadModuleMetadata(idx.modStore, modHandle.Path()) + return module.LoadModuleMetadata(ctx, idx.modStore, modHandle.Path()) }, Type: op.OpTypeLoadModuleMetadata.String(), DependsOn: dependsOn, diff --git a/internal/indexer/document_open.go b/internal/indexer/document_open.go index f6fc80c29..f6c7e3979 100644 --- a/internal/indexer/document_open.go +++ b/internal/indexer/document_open.go @@ -40,7 +40,7 @@ func (idx *Indexer) DocumentOpened(modHandle document.DirHandle) (job.IDs, error parseId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseModuleConfiguration(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseModuleConfiguration.String(), }) @@ -58,7 +58,7 @@ func (idx *Indexer) DocumentOpened(modHandle document.DirHandle) (job.IDs, error parseVarsId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseVariables(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseVariables.String(), }) diff --git a/internal/indexer/module_calls.go b/internal/indexer/module_calls.go index 6537b210b..a32755c02 100644 --- a/internal/indexer/module_calls.go +++ b/internal/indexer/module_calls.go @@ -43,7 +43,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo parseId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: mcHandle, Func: func(ctx context.Context) error { - return module.ParseModuleConfiguration(idx.fs, idx.modStore, mcPath) + return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, mcPath) }, Type: op.OpTypeParseModuleConfiguration.String(), }) @@ -60,7 +60,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo Dir: mcHandle, Type: op.OpTypeLoadModuleMetadata.String(), Func: func(ctx context.Context) error { - return module.LoadModuleMetadata(idx.modStore, mcPath) + return module.LoadModuleMetadata(ctx, idx.modStore, mcPath) }, DependsOn: job.IDs{parseId}, }) @@ -84,7 +84,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo varsParseId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: mcHandle, Func: func(ctx context.Context) error { - return module.ParseVariables(idx.fs, idx.modStore, mcPath) + return module.ParseVariables(ctx, idx.fs, idx.modStore, mcPath) }, Type: op.OpTypeParseVariables.String(), }) diff --git a/internal/indexer/walker.go b/internal/indexer/walker.go index 9eac6e323..4048b2adc 100644 --- a/internal/indexer/walker.go +++ b/internal/indexer/walker.go @@ -22,7 +22,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand parseId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseModuleConfiguration(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseModuleConfiguration.String(), }) @@ -40,7 +40,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand Dir: modHandle, Type: op.OpTypeLoadModuleMetadata.String(), Func: func(ctx context.Context) error { - return module.LoadModuleMetadata(idx.modStore, modHandle.Path()) + return module.LoadModuleMetadata(ctx, idx.modStore, modHandle.Path()) }, DependsOn: job.IDs{parseId}, }) @@ -56,7 +56,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand parseVarsId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseVariables(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseVariables.String(), }) @@ -93,7 +93,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand modManifestId, err = idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseModuleManifest(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseModuleManifest(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseModuleManifest.String(), Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index 8764385bc..ed327bfaa 100644 --- a/internal/indexer/watcher.go +++ b/internal/indexer/watcher.go @@ -17,7 +17,7 @@ func (idx *Indexer) ModuleManifestChanged(ctx context.Context, modHandle documen modManifestId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseModuleManifest(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseModuleManifest(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseModuleManifest.String(), Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { diff --git a/internal/job/ignore_state.go b/internal/job/ignore_state.go new file mode 100644 index 000000000..3bc946a97 --- /dev/null +++ b/internal/job/ignore_state.go @@ -0,0 +1,30 @@ +package job + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-ls/internal/document" +) + +type ignoreState struct{} + +func IgnoreState(ctx context.Context) bool { + v, ok := ctx.Value(ignoreState{}).(bool) + if !ok { + return false + } + return v +} + +func WithIgnoreState(ctx context.Context, ignore bool) context.Context { + return context.WithValue(ctx, ignoreState{}, ignore) +} + +type StateNotChangedErr struct { + Dir document.DirHandle +} + +func (e StateNotChangedErr) Error() string { + return fmt.Sprintf("%s: state not changed", e.Dir.URI) +} diff --git a/internal/job/job.go b/internal/job/job.go index 6ad1cc8e2..a504e752a 100644 --- a/internal/job/job.go +++ b/internal/job/job.go @@ -32,6 +32,11 @@ type Job struct { // This will be taken into account when scheduling, so that only // jobs with no dependencies are dispatched at any time. DependsOn IDs + + // IgnoreState indicates to the job (as defined by Func) + // whether to ignore existing state, i.e. whether to invalidate cache. + // It is up to [Func] to read this flag from ctx and reflect it. + IgnoreState bool } // DeferFunc represents a deferred function scheduling more jobs @@ -41,12 +46,13 @@ type DeferFunc func(ctx context.Context, jobErr error) (IDs, error) func (job Job) Copy() Job { return Job{ - Func: job.Func, - Dir: job.Dir, - Type: job.Type, - Priority: job.Priority, - Defer: job.Defer, - DependsOn: job.DependsOn.Copy(), + Func: job.Func, + Dir: job.Dir, + Type: job.Type, + Priority: job.Priority, + Defer: job.Defer, + IgnoreState: job.IgnoreState, + DependsOn: job.DependsOn.Copy(), } } diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 902f6b4f0..6d0dac976 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -65,6 +65,8 @@ func (s *Scheduler) eval(ctx context.Context) { return } + ctx = job.WithIgnoreState(ctx, nextJob.IgnoreState) + jobErr := nextJob.Func(ctx) deferredJobIds := make(job.IDs, 0) diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 7fe82141c..f11e8c916 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -18,6 +18,67 @@ import ( "github.com/hashicorp/terraform-ls/internal/state" ) +func TestScheduler_withIgnoreExistingState(t *testing.T) { + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + tmpDir := t.TempDir() + + ctx := context.Background() + + s := NewScheduler(ss.JobStore, 1, job.LowPriority) + s.SetLogger(testLogger()) + s.Start(ctx) + t.Cleanup(func() { + s.Stop() + }) + + var stateIgnored int64 = 0 + firstJobId, err := ss.JobStore.EnqueueJob(job.Job{ + Func: func(ctx context.Context) error { + if job.IgnoreState(ctx) { + atomic.AddInt64(&stateIgnored, 1) + } + return nil + }, + Dir: document.DirHandleFromPath(tmpDir), + Type: "test-type", + IgnoreState: true, + }) + if err != nil { + t.Fatal(err) + } + + var stateNotIgnored int64 = 0 + secondJobId, err := ss.JobStore.EnqueueJob(job.Job{ + Func: func(ctx context.Context) error { + if !job.IgnoreState(ctx) { + atomic.AddInt64(&stateNotIgnored, 1) + } + return nil + }, + Dir: document.DirHandleFromPath(tmpDir), + Type: "test-type", + }) + if err != nil { + t.Fatal(err) + } + + err = ss.JobStore.WaitForJobs(ctx, firstJobId, secondJobId) + if err != nil { + t.Fatal(err) + } + + if stateIgnored != 1 { + t.Fatalf("expected state to be ignored once, given: %d", stateIgnored) + } + if stateNotIgnored != 1 { + t.Fatalf("expected state not to be ignored once, given: %d", stateNotIgnored) + } +} + func TestScheduler_closedOnly(t *testing.T) { ss, err := state.NewStateStore() if err != nil { diff --git a/internal/state/jobs.go b/internal/state/jobs.go index 425a5ae83..80a79a864 100644 --- a/internal/state/jobs.go +++ b/internal/state/jobs.go @@ -103,8 +103,8 @@ func (js *JobStore) EnqueueJob(newJob job.Job) (job.ID, error) { return "", fmt.Errorf("failed to insert new job: %w", err) } - js.logger.Printf("JOBS: Enqueueing new job %q: %q for %q (IsDirOpen: %t)", - sJob.ID, sJob.Type, sJob.Dir, sJob.IsDirOpen) + js.logger.Printf("JOBS: Enqueueing new job %q: %q for %q (IsDirOpen: %t, IgnoreState: %t)", + sJob.ID, sJob.Type, sJob.Dir, sJob.IsDirOpen, sJob.IgnoreState) txn.Commit() diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index e0f7e1ebe..6cc42ca56 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -10,6 +10,8 @@ import ( "github.com/hashicorp/hcl-lang/decoder" "github.com/hashicorp/hcl-lang/lang" idecoder "github.com/hashicorp/terraform-ls/internal/decoder" + "github.com/hashicorp/terraform-ls/internal/document" + "github.com/hashicorp/terraform-ls/internal/job" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" "github.com/hashicorp/terraform-ls/internal/registry" "github.com/hashicorp/terraform-ls/internal/state" @@ -58,6 +60,11 @@ func GetTerraformVersion(ctx context.Context, modStore *state.ModuleStore, modPa return err } + // Avoid getting version if getting is already in progress or already known + if mod.TerraformVersionState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + err = modStore.SetTerraformVersionState(modPath, op.OpStateLoading) if err != nil { return err @@ -103,6 +110,16 @@ func providerVersionsFromTfVersion(pv map[string]*version.Version) map[tfaddr.Pr } func ObtainSchema(ctx context.Context, modStore *state.ModuleStore, schemaStore *state.ProviderSchemaStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid obtaining schema if it is already in progress or already known + if mod.ProviderSchemaState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + pReqs, err := modStore.ProviderRequirementsForModule(modPath) if err != nil { return err @@ -157,8 +174,18 @@ func ObtainSchema(ctx context.Context, modStore *state.ModuleStore, schemaStore return nil } -func ParseModuleConfiguration(fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { - err := modStore.SetModuleParsingState(modPath, op.OpStateLoading) +func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid parsing if it is already in progress or already known + if mod.ModuleParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetModuleParsingState(modPath, op.OpStateLoading) if err != nil { return err } @@ -178,8 +205,18 @@ func ParseModuleConfiguration(fs ReadOnlyFS, modStore *state.ModuleStore, modPat return err } -func ParseVariables(fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { - err := modStore.SetVarsParsingState(modPath, op.OpStateLoading) +func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid parsing if it is already in progress or already known + if mod.VarsParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetVarsParsingState(modPath, op.OpStateLoading) if err != nil { return err } @@ -199,8 +236,18 @@ func ParseVariables(fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) return err } -func ParseModuleManifest(fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { - err := modStore.SetModManifestState(modPath, op.OpStateLoading) +func ParseModuleManifest(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid parsing if it is already in progress or already known + if mod.ModManifestState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetModManifestState(modPath, op.OpStateLoading) if err != nil { return err } @@ -249,13 +296,18 @@ func ParseProviderVersions(fs ReadOnlyFS, modStore *state.ModuleStore, modPath s return err } -func LoadModuleMetadata(modStore *state.ModuleStore, modPath string) error { - err := modStore.SetMetaState(modPath, op.OpStateLoading) +func LoadModuleMetadata(ctx context.Context, modStore *state.ModuleStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) if err != nil { return err } - mod, err := modStore.ModuleByPath(modPath) + // Avoid parsing if it is already in progress or already known + if mod.MetaState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetMetaState(modPath, op.OpStateLoading) if err != nil { return err } @@ -288,7 +340,17 @@ func LoadModuleMetadata(modStore *state.ModuleStore, modPath string) error { } func DecodeReferenceTargets(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { - err := modStore.SetReferenceTargetsState(modPath, op.OpStateLoading) + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid collection if it is already in progress or already done + if mod.RefTargetsState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetReferenceTargetsState(modPath, op.OpStateLoading) if err != nil { return err } @@ -319,7 +381,17 @@ func DecodeReferenceTargets(ctx context.Context, modStore *state.ModuleStore, sc } func DecodeReferenceOrigins(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { - err := modStore.SetReferenceOriginsState(modPath, op.OpStateLoading) + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid collection if it is already in progress or already done + if mod.RefOriginsState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetReferenceOriginsState(modPath, op.OpStateLoading) if err != nil { return err } @@ -349,7 +421,17 @@ func DecodeReferenceOrigins(ctx context.Context, modStore *state.ModuleStore, sc } func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { - err := modStore.SetVarsReferenceOriginsState(modPath, op.OpStateLoading) + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid collection if it is already in progress or already done + if mod.VarsRefOriginsState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetVarsReferenceOriginsState(modPath, op.OpStateLoading) if err != nil { return err } diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 7bac65f80..5839dbe44 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -43,12 +43,12 @@ func TestGetModuleDataFromRegistry_singleModule(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) - err = ParseModuleConfiguration(fs, ss.Modules, modPath) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) if err != nil { t.Fatal(err) } - err = LoadModuleMetadata(ss.Modules, modPath) + err = LoadModuleMetadata(ctx, ss.Modules, modPath) if err != nil { t.Fatal(err) } @@ -115,12 +115,12 @@ func TestGetModuleDataFromRegistry_moduleNotFound(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) - err = ParseModuleConfiguration(fs, ss.Modules, modPath) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) if err != nil { t.Fatal(err) } - err = LoadModuleMetadata(ss.Modules, modPath) + err = LoadModuleMetadata(ctx, ss.Modules, modPath) if err != nil { t.Fatal(err) } @@ -194,12 +194,12 @@ func TestGetModuleDataFromRegistry_apiTimeout(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) - err = ParseModuleConfiguration(fs, ss.Modules, modPath) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) if err != nil { t.Fatal(err) } - err = LoadModuleMetadata(ss.Modules, modPath) + err = LoadModuleMetadata(ctx, ss.Modules, modPath) if err != nil { t.Fatal(err) } From 1ac6cae194aee9e75a316579679b97bdf528257b Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 8 Aug 2022 12:50:07 +0100 Subject: [PATCH 3/7] state: Avoid deduplicating jobs on enqueuing --- internal/state/jobs.go | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/internal/state/jobs.go b/internal/state/jobs.go index 80a79a864..26a7ae8ea 100644 --- a/internal/state/jobs.go +++ b/internal/state/jobs.go @@ -58,22 +58,6 @@ const ( ) func (js *JobStore) EnqueueJob(newJob job.Job) (job.ID, error) { - jobID, queued, err := js.jobExists(newJob, StateQueued) - if err != nil { - return "", err - } - if queued { - return jobID, nil - } - - jobID, running, err := js.jobExists(newJob, StateRunning) - if err != nil { - return "", err - } - if running { - return jobID, nil - } - txn := js.db.Txn(true) defer txn.Abort() @@ -98,7 +82,7 @@ func (js *JobStore) EnqueueJob(newJob job.Job) (job.ID, error) { State: StateQueued, } - err = txn.Insert(js.tableName, sJob) + err := txn.Insert(js.tableName, sJob) if err != nil { return "", fmt.Errorf("failed to insert new job: %w", err) } From 15f0482538f7d4087b4de1efc92f49e289ce8163 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Fri, 5 Aug 2022 20:29:16 +0100 Subject: [PATCH 4/7] indexer: use IgnoreState --- internal/indexer/document_change.go | 30 ++++++++++-------- internal/indexer/document_open.go | 8 +++-- internal/indexer/module_calls.go | 30 ++++++++++-------- internal/indexer/walker.go | 6 ++-- internal/indexer/watcher.go | 15 +++++---- internal/terraform/module/module_ops.go | 32 ++++++++++++++++++-- internal/terraform/module/module_ops_test.go | 3 +- 7 files changed, 85 insertions(+), 39 deletions(-) diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 84c237f2e..03552e80b 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -17,14 +17,15 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro Func: func(ctx context.Context) error { return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeParseModuleConfiguration.String(), + Type: op.OpTypeParseModuleConfiguration.String(), + IgnoreState: true, }) if err != nil { return ids, err } ids = append(ids, parseId) - modIds, err := idx.decodeModule(modHandle, job.IDs{parseId}) + modIds, err := idx.decodeModule(modHandle, job.IDs{parseId}, true) if err != nil { return ids, err } @@ -35,7 +36,8 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro Func: func(ctx context.Context) error { return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeParseVariables.String(), + Type: op.OpTypeParseVariables.String(), + IgnoreState: true, }) if err != nil { return ids, err @@ -47,8 +49,9 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro Func: func(ctx context.Context) error { return module.DecodeVarsReferences(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeDecodeVarsReferences.String(), - DependsOn: job.IDs{parseVarsId}, + Type: op.OpTypeDecodeVarsReferences.String(), + DependsOn: job.IDs{parseVarsId}, + IgnoreState: true, }) if err != nil { return ids, err @@ -58,7 +61,7 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro return ids, nil } -func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs) (job.IDs, error) { +func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs, ignoreState bool) (job.IDs, error) { ids := make(job.IDs, 0) metaId, err := idx.jobStore.EnqueueJob(job.Job{ @@ -66,8 +69,9 @@ func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs Func: func(ctx context.Context) error { return module.LoadModuleMetadata(ctx, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeLoadModuleMetadata.String(), - DependsOn: dependsOn, + Type: op.OpTypeLoadModuleMetadata.String(), + DependsOn: dependsOn, + IgnoreState: ignoreState, }) if err != nil { return ids, err @@ -79,8 +83,9 @@ func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs Func: func(ctx context.Context) error { return module.DecodeReferenceTargets(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeDecodeReferenceTargets.String(), - DependsOn: job.IDs{metaId}, + Type: op.OpTypeDecodeReferenceTargets.String(), + DependsOn: job.IDs{metaId}, + IgnoreState: ignoreState, }) if err != nil { return ids, err @@ -92,8 +97,9 @@ func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs Func: func(ctx context.Context) error { return module.DecodeReferenceOrigins(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeDecodeReferenceOrigins.String(), - DependsOn: job.IDs{metaId}, + Type: op.OpTypeDecodeReferenceOrigins.String(), + DependsOn: job.IDs{metaId}, + IgnoreState: ignoreState, }) if err != nil { return ids, err diff --git a/internal/indexer/document_open.go b/internal/indexer/document_open.go index f6c7e3979..bcd3902c8 100644 --- a/internal/indexer/document_open.go +++ b/internal/indexer/document_open.go @@ -42,14 +42,15 @@ func (idx *Indexer) DocumentOpened(modHandle document.DirHandle) (job.IDs, error Func: func(ctx context.Context) error { return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeParseModuleConfiguration.String(), + Type: op.OpTypeParseModuleConfiguration.String(), + IgnoreState: true, }) if err != nil { return ids, err } ids = append(ids, parseId) - modIds, err := idx.decodeModule(modHandle, job.IDs{parseId}) + modIds, err := idx.decodeModule(modHandle, job.IDs{parseId}, true) if err != nil { return ids, err } @@ -60,7 +61,8 @@ func (idx *Indexer) DocumentOpened(modHandle document.DirHandle) (job.IDs, error Func: func(ctx context.Context) error { return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeParseVariables.String(), + Type: op.OpTypeParseVariables.String(), + IgnoreState: true, }) if err != nil { return ids, err diff --git a/internal/indexer/module_calls.go b/internal/indexer/module_calls.go index a32755c02..960def83f 100644 --- a/internal/indexer/module_calls.go +++ b/internal/indexer/module_calls.go @@ -11,7 +11,7 @@ import ( op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" ) -func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (job.IDs, error) { +func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle, ignoreState bool) (job.IDs, error) { jobIds := make(job.IDs, 0) moduleCalls, err := idx.modStore.ModuleCalls(modHandle.Path()) @@ -45,7 +45,8 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo Func: func(ctx context.Context) error { return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, mcPath) }, - Type: op.OpTypeParseModuleConfiguration.String(), + Type: op.OpTypeParseModuleConfiguration.String(), + IgnoreState: ignoreState, }) if err != nil { multierror.Append(errs, err) @@ -62,7 +63,8 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo Func: func(ctx context.Context) error { return module.LoadModuleMetadata(ctx, idx.modStore, mcPath) }, - DependsOn: job.IDs{parseId}, + DependsOn: job.IDs{parseId}, + IgnoreState: ignoreState, }) if err != nil { multierror.Append(errs, err) @@ -73,7 +75,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo } if parseId != "" { - ids, err := idx.collectReferences(mcHandle, refCollectionDeps) + ids, err := idx.collectReferences(mcHandle, refCollectionDeps, ignoreState) if err != nil { multierror.Append(errs, err) } else { @@ -86,7 +88,8 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo Func: func(ctx context.Context) error { return module.ParseVariables(ctx, idx.fs, idx.modStore, mcPath) }, - Type: op.OpTypeParseVariables.String(), + Type: op.OpTypeParseVariables.String(), + IgnoreState: ignoreState, }) if err != nil { multierror.Append(errs, err) @@ -100,8 +103,9 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo Func: func(ctx context.Context) error { return module.DecodeVarsReferences(ctx, idx.modStore, idx.schemaStore, mcPath) }, - Type: op.OpTypeDecodeVarsReferences.String(), - DependsOn: job.IDs{varsParseId}, + Type: op.OpTypeDecodeVarsReferences.String(), + DependsOn: job.IDs{varsParseId}, + IgnoreState: ignoreState, }) if err != nil { multierror.Append(errs, err) @@ -114,7 +118,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo return jobIds, errs.ErrorOrNil() } -func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn job.IDs) (job.IDs, error) { +func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn job.IDs, ignoreState bool) (job.IDs, error) { ids := make(job.IDs, 0) var errs *multierror.Error @@ -124,8 +128,9 @@ func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn jo Func: func(ctx context.Context) error { return module.DecodeReferenceTargets(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeDecodeReferenceTargets.String(), - DependsOn: dependsOn, + Type: op.OpTypeDecodeReferenceTargets.String(), + DependsOn: dependsOn, + IgnoreState: ignoreState, }) if err != nil { errs = multierror.Append(errs, err) @@ -138,8 +143,9 @@ func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn jo Func: func(ctx context.Context) error { return module.DecodeReferenceOrigins(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeDecodeReferenceOrigins.String(), - DependsOn: dependsOn, + Type: op.OpTypeDecodeReferenceOrigins.String(), + DependsOn: dependsOn, + IgnoreState: ignoreState, }) if err != nil { errs = multierror.Append(errs, err) diff --git a/internal/indexer/walker.go b/internal/indexer/walker.go index 4048b2adc..7a6cb1af8 100644 --- a/internal/indexer/walker.go +++ b/internal/indexer/walker.go @@ -97,7 +97,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand }, Type: op.OpTypeParseModuleManifest.String(), Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { - return idx.decodeInstalledModuleCalls(modHandle) + return idx.decodeInstalledModuleCalls(modHandle, false) }, }) if err != nil { @@ -114,7 +114,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand pSchemaVerId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseProviderVersions(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseProviderVersions(ctx, idx.fs, idx.modStore, modHandle.Path()) }, Type: op.OpTypeParseProviderVersions.String(), DependsOn: providerVersionDeps, @@ -144,7 +144,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand } if parseId != "" { - rIds, err := idx.collectReferences(modHandle, refCollectionDeps) + rIds, err := idx.collectReferences(modHandle, refCollectionDeps, false) if err != nil { errs = multierror.Append(errs, err) } else { diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index ed327bfaa..125336fa3 100644 --- a/internal/indexer/watcher.go +++ b/internal/indexer/watcher.go @@ -19,9 +19,10 @@ func (idx *Indexer) ModuleManifestChanged(ctx context.Context, modHandle documen Func: func(ctx context.Context) error { return module.ParseModuleManifest(ctx, idx.fs, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeParseModuleManifest.String(), + Type: op.OpTypeParseModuleManifest.String(), + IgnoreState: true, Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { - return idx.decodeInstalledModuleCalls(modHandle) + return idx.decodeInstalledModuleCalls(modHandle, true) }, }) if err != nil { @@ -39,9 +40,10 @@ func (idx *Indexer) PluginLockChanged(ctx context.Context, modHandle document.Di pSchemaVerId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.ParseProviderVersions(idx.fs, idx.modStore, modHandle.Path()) + return module.ParseProviderVersions(ctx, idx.fs, idx.modStore, modHandle.Path()) }, - Type: op.OpTypeParseProviderVersions.String(), + IgnoreState: true, + Type: op.OpTypeParseProviderVersions.String(), }) if err != nil { errs = multierror.Append(errs, err) @@ -55,8 +57,9 @@ func (idx *Indexer) PluginLockChanged(ctx context.Context, modHandle document.Di ctx = exec.WithExecutorFactory(ctx, idx.tfExecFactory) return module.ObtainSchema(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeObtainSchema.String(), - DependsOn: job.IDs{pSchemaVerId}, + IgnoreState: true, + Type: op.OpTypeObtainSchema.String(), + DependsOn: job.IDs{pSchemaVerId}, }) if err != nil { errs = multierror.Append(errs, err) diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 6cc42ca56..067040633 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -180,6 +180,10 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat return err } + // TODO: Avoid parsing if the content matches existing AST + + // TODO: Only parse the file that's being changed/opened, unless this is 1st-time parsing + // Avoid parsing if it is already in progress or already known if mod.ModuleParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} @@ -211,6 +215,10 @@ func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleSt return err } + // TODO: Avoid parsing if the content matches existing AST + + // TODO: Only parse the file that's being changed/opened, unless this is 1st-time parsing + // Avoid parsing if it is already in progress or already known if mod.VarsParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} @@ -280,8 +288,18 @@ func ParseModuleManifest(ctx context.Context, fs ReadOnlyFS, modStore *state.Mod return err } -func ParseProviderVersions(fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { - err := modStore.SetInstalledProvidersState(modPath, op.OpStateLoading) +func ParseProviderVersions(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid parsing if it is already in progress or already known + if mod.InstalledProvidersState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetInstalledProvidersState(modPath, op.OpStateLoading) if err != nil { return err } @@ -302,6 +320,8 @@ func LoadModuleMetadata(ctx context.Context, modStore *state.ModuleStore, modPat return err } + // TODO: Avoid parsing if upstream (parsing) job reported no changes + // Avoid parsing if it is already in progress or already known if mod.MetaState != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} @@ -345,6 +365,8 @@ func DecodeReferenceTargets(ctx context.Context, modStore *state.ModuleStore, sc return err } + // TODO: Avoid collection if upstream jobs reported no changes + // Avoid collection if it is already in progress or already done if mod.RefTargetsState != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} @@ -386,6 +408,8 @@ func DecodeReferenceOrigins(ctx context.Context, modStore *state.ModuleStore, sc return err } + // TODO: Avoid collection if upstream jobs reported no changes + // Avoid collection if it is already in progress or already done if mod.RefOriginsState != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} @@ -426,6 +450,8 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche return err } + // TODO: Avoid collection if upstream (parsing) job reported no changes + // Avoid collection if it is already in progress or already done if mod.VarsRefOriginsState != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} @@ -466,6 +492,8 @@ func GetModuleDataFromRegistry(ctx context.Context, regClient registry.Client, m return err } + // TODO: Avoid collection if upstream jobs (parsing, meta) reported no changes + var errs *multierror.Error for _, declaredModule := range calls.Declared { diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 5839dbe44..b87c8fa16 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -368,7 +368,8 @@ func TestParseProviderVersions(t *testing.T) { t.Fatal(err) } - err = ParseProviderVersions(fs, ss.Modules, modPath) + ctx := context.Background() + err = ParseProviderVersions(ctx, fs, ss.Modules, modPath) if err != nil { t.Fatal(err) } From 0111077c1909ae1060730f47ccb4c2f24a8fdcba Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 9 Aug 2022 10:01:18 +0100 Subject: [PATCH 5/7] ci: bump benchmarks total timeout --- .github/workflows/benchmarks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 4191ddf41..e16c5adb4 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -41,7 +41,7 @@ jobs: -bench=InitializeFolder_basic \ -run=^# \ -benchtime=60s \ - -timeout=30m | tee ${{ runner.temp }}/benchmarks.txt + -timeout=60m | tee ${{ runner.temp }}/benchmarks.txt - name: Evaluate benchmarks id: bench-eval From d425b9a9de71d04cd30bcbbd613edb99815ace2b Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 9 Aug 2022 11:25:40 +0100 Subject: [PATCH 6/7] update benchmarks thresholds --- .github/gobenchdata-checks.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/gobenchdata-checks.yml b/.github/gobenchdata-checks.yml index 43cb38646..5a4a2b00e 100644 --- a/.github/gobenchdata-checks.yml +++ b/.github/gobenchdata-checks.yml @@ -4,21 +4,21 @@ checks: benchmarks: [BenchmarkInitializeFolder_basic/local-single-module-no-provider] diff: current.NsPerOp / 1000000 # ms thresholds: - min: 25 - max: 55 + min: 3 + max: 20 - package: ./internal/langserver/handlers name: local-single-submodule-no-provider benchmarks: [BenchmarkInitializeFolder_basic/local-single-submodule-no-provider] diff: current.NsPerOp / 1000000 # ms thresholds: - min: 140 - max: 310 + min: 100 + max: 250 - package: ./internal/langserver/handlers name: local-single-module-random benchmarks: [BenchmarkInitializeFolder_basic/local-single-module-random] diff: current.NsPerOp / 1000000 # ms thresholds: - min: 140 + min: 100 max: 300 - package: ./internal/langserver/handlers name: local-single-module-aws @@ -47,35 +47,35 @@ checks: diff: current.NsPerOp / 1000000 # ms thresholds: min: 1400 - max: 2200 + max: 5000 - package: ./internal/langserver/handlers name: google-project benchmarks: [BenchmarkInitializeFolder_basic/google-project] diff: current.NsPerOp / 1000000 # ms thresholds: min: 1570 - max: 2450 + max: 9000 - package: ./internal/langserver/handlers name: google-network benchmarks: [BenchmarkInitializeFolder_basic/google-network] diff: current.NsPerOp / 1000000 # ms thresholds: min: 1430 - max: 2700 + max: 15000 - package: ./internal/langserver/handlers name: google-gke benchmarks: [BenchmarkInitializeFolder_basic/google-gke] diff: current.NsPerOp / 1000000 # ms thresholds: min: 1500 - max: 5100 + max: 20000 - package: ./internal/langserver/handlers name: k8s-metrics-server benchmarks: [BenchmarkInitializeFolder_basic/k8s-metrics-server] diff: current.NsPerOp / 1000000 # ms thresholds: min: 1000 - max: 3200 + max: 4000 - package: ./internal/langserver/handlers name: k8s-dashboard benchmarks: [BenchmarkInitializeFolder_basic/k8s-dashboard] From a2fa3a048446ec24bbb3086e79553c247cc39f3b Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 10 Aug 2022 14:40:32 +0100 Subject: [PATCH 7/7] indexer: run ObtainSchema even if scheduling of an upstream job fails --- internal/indexer/walker.go | 4 +++- internal/indexer/watcher.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/indexer/walker.go b/internal/indexer/walker.go index 7a6cb1af8..94d633a4a 100644 --- a/internal/indexer/walker.go +++ b/internal/indexer/walker.go @@ -111,6 +111,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand } if dataDir.PluginLockFilePath != "" { + dependsOn := make(job.IDs, 0) pSchemaVerId, err := idx.jobStore.EnqueueJob(job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { @@ -123,6 +124,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand errs = multierror.Append(errs, err) } else { ids = append(ids, pSchemaVerId) + dependsOn = append(dependsOn, pSchemaVerId) refCollectionDeps = append(refCollectionDeps, pSchemaVerId) } @@ -133,7 +135,7 @@ func (idx *Indexer) WalkedModule(ctx context.Context, modHandle document.DirHand return module.ObtainSchema(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, Type: op.OpTypeObtainSchema.String(), - DependsOn: job.IDs{pSchemaVerId}, + DependsOn: dependsOn, }) if err != nil { errs = multierror.Append(errs, err) diff --git a/internal/indexer/watcher.go b/internal/indexer/watcher.go index 125336fa3..9142c2576 100644 --- a/internal/indexer/watcher.go +++ b/internal/indexer/watcher.go @@ -35,6 +35,7 @@ func (idx *Indexer) ModuleManifestChanged(ctx context.Context, modHandle documen func (idx *Indexer) PluginLockChanged(ctx context.Context, modHandle document.DirHandle) (job.IDs, error) { ids := make(job.IDs, 0) + dependsOn := make(job.IDs, 0) var errs *multierror.Error pSchemaVerId, err := idx.jobStore.EnqueueJob(job.Job{ @@ -49,6 +50,7 @@ func (idx *Indexer) PluginLockChanged(ctx context.Context, modHandle document.Di errs = multierror.Append(errs, err) } else { ids = append(ids, pSchemaVerId) + dependsOn = append(dependsOn, pSchemaVerId) } pSchemaId, err := idx.jobStore.EnqueueJob(job.Job{ @@ -59,7 +61,7 @@ func (idx *Indexer) PluginLockChanged(ctx context.Context, modHandle document.Di }, IgnoreState: true, Type: op.OpTypeObtainSchema.String(), - DependsOn: job.IDs{pSchemaVerId}, + DependsOn: dependsOn, }) if err != nil { errs = multierror.Append(errs, err)