-
Notifications
You must be signed in to change notification settings - Fork 137
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
Cap root module loading via worker pool #256
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package handlers | ||
|
||
import ( | ||
"context" | ||
"time" | ||
) | ||
|
||
func newTickReporter(d time.Duration) *tickReporter { | ||
return &tickReporter{ | ||
t: time.NewTicker(d), | ||
rfs: make([]reportFunc, 0), | ||
} | ||
} | ||
|
||
type reportFunc func() | ||
|
||
type tickReporter struct { | ||
t *time.Ticker | ||
rfs []reportFunc | ||
} | ||
|
||
func (tr *tickReporter) AddReporter(f reportFunc) { | ||
tr.rfs = append(tr.rfs, f) | ||
} | ||
|
||
func (tr *tickReporter) StartReporting(ctx context.Context) { | ||
go func(ctx context.Context, tr *tickReporter) { | ||
for { | ||
select { | ||
case <-ctx.Done(): | ||
tr.t.Stop() | ||
return | ||
case <-tr.t.C: | ||
for _, rf := range tr.rfs { | ||
rf() | ||
} | ||
} | ||
} | ||
}(ctx, tr) | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
3 is the magic number I'm guessing?
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.
3 is basically an educated guess. I did some simple testing as shown above with 1000 root modules in a hierarchy (which seemed to be the higher end of range reported by folks in #186 ). That effectively triggers
terraform version
1000 times and I ran that on my i7 (Macbook) with4
cores (= pool size12
) and it finished within a few seconds.I don't expect this to be the best configuration nor the best way of testing this, but I do expect it to behave better than before just because we trade time+memory for less CPU in principle.
I do expect we will tweak the number in the near future based on the feedback from end users and further (more scoped) testing. This is also why I added the logging.
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.
Generally the performance will largely depend on the time it takes to finish the loading, which after
terraform version
depends on the size of the schema and exec time ofterraform providers schema -json
alone may span from half a second to 10+ seconds in some worst case scenarios, so there is no easy answer to this I'm afraid.We can however make it configurable and let users deal with edge cases (too slow CPU and/or too many modules) that way. Also it would be even better to just avoid calling
terraform version
where we don't need to as discussed in #186 (comment)