walker: Use memdb for queued paths

[radeksimko]

Fixes #838

The bug was originally introduced in #502 - i.e. has been around since introduction of workspace support, for about 9 months. I am therefore leaning towards just slotting this into a regular release schedule (as opposed to cutting patch release).

---

Previously we stored queued paths in a priority queue implemented via container/heap. To allow a more efficient synchronization (which is needed in tests) and a little more readable code (at least the walker code), the data was moved into a new memdb table. This means we can leverage memdb watch channels to do the synchronization and entirely avoid a separate "SyncWalker" implementation just for tests.

New walker_paths memdb table

Each entry within the new table is represented as WalkerPath:

terraform-ls/internal/state/walker_paths.go

Lines 23 to 27 in 898694f

	type WalkerPath struct {
	Dir document.DirHandle
	IsDirOpen bool
	State PathState
	}

WalkerCollector

A new WalkerCollector is introduced to collect job IDs and errors as part of walking, such that we can block & wait for completion of all these async jobs in tests and also fail tests if errors occur during walking.

terraform-ls/internal/terraform/module/walker_collector.go

Lines 10 to 16 in 898694f

	type WalkerCollector struct {
	errors *multierror.Error
	errorsMu *sync.RWMutex
	jobIds job.IDs
	jobIdsMu *sync.RWMutex
	}

Synchronization in tests

A new helper function is introduced to block and wait for:

1. walker to finish walking a given path
2. jobs scheduled from that walk to finish

and also to flag up any errors occurred during the walk as test failures.

terraform-ls/internal/langserver/handlers/handlers_test.go

Lines 83 to 97 in 898694f

	func waitForWalkerPath(t *testing.T, ss *state.StateStore, wc *module.WalkerCollector, dir document.DirHandle) {
	ctx := context.Background()
	err := ss.WalkerPaths.WaitForDirs(ctx, []document.DirHandle{dir})
	if err != nil {
	t.Fatal(err)
	}
	err = ss.JobStore.WaitForJobs(ctx, wc.JobIds()...)
	if err != nil {
	t.Fatal(err)
	}
	err = wc.ErrorOrNil()
	if err != nil {
	t.Fatal(err)
	}
	}

This replaces the clunkier sync mode of the walker where StartWalk() would optionally block under test.

---

Future - General Purpose Walker?

As noted with the // TODO inline comment, I see an opportunity to further cleanup the Walker code and make it more general-purpose by extracting the logic that runs for every module found. That would likely allow us to remove most of the arguments from NewWalker. I just didn't want to do it in this PR which already got quite big.

[radeksimko]

Converting to draft as there is still ongoing issue with the channel being filled up due to the fact that we only consume from the channel on a certain occasion.

terraform-ls/internal/terraform/module/walker.go

Lines 191 to 204 in bc219ad

	if w.queue.Len() == 0 {
	w.queueMu.Unlock()
	select {
	case <-w.pushChan:
	// block to avoid infinite loop
	continue
	case <-w.doneCh:
	return
	}
	}
	nextPath := heap.Pop(w.queue)
	w.queueMu.Unlock()
	path := nextPath.(string)
	nextPathToWalk <- path

This would suggest that walker is basically capped 50 paths all the time.

I want to explore the possibility of moving the path queue into memdb as well since the problem we have here is basically the same as in the job scheduler and it might allow us to get rid of most sync logic within the walker.

[radeksimko]

I have a draft of a solution (pushed to branch) but it's not fully finished and it looks like it would be best to first tackle #515

[dbanck]

👍 thanks for the helpful PR description!

I've only a small suggestion, but that isn't a blocker

[github-actions]

This functionality has been released in v0.27.0 of the language server.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.