-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
perf: ⚡️ run apply namespaces in apply pipeline concurrently #644
Conversation
9815c3d
to
c400454
Compare
@@ -157,12 +157,6 @@ func (m *ApplierImpl) TerraformInitAndPlan(namespace, directory string) (*tfjson | |||
return nil, "", errors.New("unable to do Terraform Plan: " + err.Error()) | |||
} | |||
|
|||
// ignore if any changes or no changes. | |||
_, err = terraform.Plan(context.Background()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicated plan see line 152 above
return err | ||
} | ||
done := make(chan bool) | ||
defer close(done) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
close the channel which cascades down through the child go routines and in turn closes them to prevent memory leaks
done := make(chan bool) | ||
defer close(done) | ||
|
||
chunkStream := util.Generator(done, chunkFolder...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convert array into a channel
|
||
routineResults := a.parallelApplyNamespace(done, chunkStream) | ||
|
||
results := util.FanIn(done, routineResults...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combine multiple channel results back into a single channel
results := make(chan string) | ||
go func() { | ||
defer close(results) | ||
for dir := range dirStream { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a new go routine run each apply for each namespace
return nil | ||
} | ||
|
||
func (a *Apply) parallelApplyNamespace(done <-chan bool, dirStream <-chan string) []<-chan string { | ||
numRoutines := runtime.NumCPU() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run as many go routines as we have cores, this proves to be a conservative number of routines because of how lightweight and cheap they are. We should monitor performance in concourse and adjust here if necessary.
f1e6012
to
96dfd2c
Compare
8090894
to
6db65c3
Compare
6db65c3
to
c73ad35
Compare
Continue chunking and batching namespaces across different pipelines but within those pipelines run the applies concurrently.
This PR covers:
destroyNamespace
,planNamespace
andapplyNamespace
to fix a bug where incorrectly structs were accessed during parallel runsdeleteutils
toutil
performance results:
https://concourse.cloud-platform.service.justice.gov.uk/teams/main/pipelines/environments-live/jobs/apply-namespace-parallel-test/builds/2 (one errored namespace can be found at the bottom of the log)
build peaked (running on worker-3) Another apply pipeline was running at the same time on the same worker, but another was running on worker 1 at the same time
Note current pod limits are (if we raise the limits go will utilise those extra resources):