Skip to content

fix: parallel_plan and parallel_apply #6427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nvanheuverzwijn wants to merge 1 commit into from
+0 −4
Closed

fix: parallel_plan and parallel_apply #6427

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions server/events/project_command_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,10 +830,6 @@ func (p *DefaultProjectCommandRunner) doStateRm(ctx command.ProjectContext) (out
func (p *DefaultProjectCommandRunner) runSteps(steps []valid.Step, ctx command.ProjectContext, absPath string) ([]string, error) {
var outputs []string

Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the WorkingDir.GitReadLock() around step execution means Clone()/MergeAgain() (which take a git write lock and can reset/merge the workspace) may now run concurrently with terraform steps in the same PR/workspace. That can mutate the working tree mid-plan/apply, leading to inconsistent results or intermittent failures under parallel execution.

Consider keeping a shared read lock while running steps, and instead addressing the parallelism bottleneck by changing WorkingDir.Clone/MergeAgain to avoid taking the write lock when they are effectively no-ops (e.g., acquire a read lock to check whether the workspace is already at the desired ref and only upgrade to a write lock when an update/merge is actually needed), or by cloning/merging once per PR/workspace before starting parallel project workers.

Suggested change
unlock := ctx.WorkingDir.GitReadLock()
defer unlock()

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@nvanheuverzwijn nvanheuverzwijn Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I beleive we already get a proper lock with

	// Acquire internal lock for the directory we're going to operate in.
	unlockFn, err := p.WorkingDirLocker.TryLock(ctx.Pull.BaseRepo.FullName, ctx.Pull.Num, ctx.Workspace, ctx.RepoRelDir, ctx.ProjectName, command.Plan)
	if err != nil {
		return nil, "", err
	}
	defer unlockFn()

in doPlan and doApply.

// Hold a read lock for the whole step run so clone/reset/merge cannot run in this dir until we're done.
unlock := p.WorkingDir.GitReadLock(ctx.Pull.BaseRepo, ctx.Pull, ctx.Workspace)
defer unlock()

envs := make(map[string]string)
for _, step := range steps {
var out string
Expand Down
Loading