Skip to content

chore: fix race condition on aggregating terraform logs #21067

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

Jump to bottom
Open
Emyrk wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: fix race condition on aggregating terraform logs #21067

Emyrk wants to merge 2 commits into from
+7 −8

Conversation

@Emyrk
Copy link
Member

@Emyrk Emyrk commented Dec 2, 2025 •
edited
Loading

I noticed we have a defer to make sure all log output is captured by the async log handling routine.

	defer func() {
		_ = outWriter.Close()
		_ = errWriter.Close()
		<-doneOut
		<-doneErr
	}()

But e.timings.aggregate() compiles the current parsed logs. I was getting some race conditions in testing if the logs did not completely parse before aggregate was called.

This was referenced Dec 2, 2025
Open
Open
@Emyrk Graphite App
Copy link
Member Author

Emyrk commented Dec 2, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Emyrk Emyrk mentioned this pull request Dec 2, 2025
Merged
@Emyrk Emyrk changed the base branch from stevenmasley/provisioner_script_shebangs to graphite-base/21067 December 2, 2025 19:03
@Emyrk Emyrk force-pushed the graphite-base/21067 branch from d0c722d to c3c059f Compare December 2, 2025 19:03
@Emyrk Emyrk force-pushed the stevenmasley/provisioner_script_race branch from 4268dd7 to b1f75f3 Compare December 2, 2025 19:03
@graphite-app graphite-app bot changed the base branch from graphite-base/21067 to main December 2, 2025 19:04
@Emyrk Emyrk force-pushed the stevenmasley/provisioner_script_race branch from b1f75f3 to eed2f12 Compare December 2, 2025 19:04
@Emyrk Emyrk marked this pull request as ready for review December 2, 2025 19:13
geokat
geokat reviewed Dec 2, 2025
View reviewed changes
@Emyrk Emyrk force-pushed the stevenmasley/provisioner_script_race branch from eed2f12 to 56dffc9 Compare December 3, 2025 12:04
geokat
geokat reviewed Dec 6, 2025
View reviewed changes
@Emyrk Emyrk force-pushed the stevenmasley/provisioner_script_race branch 2 times, most recently from cf94b5d to 6b8d466 Compare December 9, 2025 15:23
@Emyrk Emyrk mentioned this pull request Dec 11, 2025
Open
@Emyrk Emyrk force-pushed the stevenmasley/provisioner_script_race branch from 6b8d466 to f06c861 Compare December 12, 2025 13:45
@Emyrk Emyrk force-pushed the stevenmasley/provisioner_script_race branch from f06c861 to c905ed3 Compare December 12, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@geokat geokat geokat left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Emyrk Emyrk

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Emyrk @geokat