-
Notifications
You must be signed in to change notification settings - Fork 131
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
Set a cap for auto-loading root modules #186
Comments
If this is what's launching hundreds of calls to terraform version, then the UX is awful. My machine is useless until this finishes. |
Just out of curiosity do you use That aside I think we could make the discovery a bit smarter by avoid calling |
I'm seeing the same as @yruss972 with far fewer modules, but I'm using terragrunt |
@phekmat Do you mind translating "fewer" into more tangible quantity for me, so we have a more accurate data point to work with? Also I'm not that familiar with Terragrunt, but I'm assuming it doesn't install/manage Terraform itself, so I'm assuming there's still a chance that you may use tfenv with terragrunt -> do you? |
I don't use tfenv. My directory structure is roughly:
I have ~25 modules (with a couple of inter-dependencies) and each So while I technically have fewer modules, that creates a ~4000 module directories in the worst case (of having run terragrunt recently across all of my environments). I think my issue in particular may be solved by hashicorp/vscode-terraform#169 and hashicorp/vscode-terraform#405 (I already have the excludes mentioned in the former). I also just noticed the new "root modules" setting in VSCode, so that may help my case too. |
Thank you for sharing these details. This is helpful. Re hashicorp/vscode-terraform#405 - I don't think that resolving this issue specifically will have any significant impact on CPU in particular. The issue aims to solve a different problem - which is flooding the server with irrelevant Re hashicorp/vscode-terraform#169 - This is actually an issue from an older version of the extension and/or the other language server, but you are right that giving users the option to exclude paths, especially if terragrunt works the way you describe is sensible. I have created #250 to track that.
That's right - this should help you pick particular directories and bypass the auto-discovery entirely. |
No tfenv and root modules setting in vscode is configured for all the projects so possibly my experience is not related to the auto-discovery but just the refresh of some metadata? |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Current Version
Use-cases
Currently the language server will attempt to load all root modules (basically directories with
.terraform
in them) oninitialize
. This is probably fine for most users which have sensible amount of root modules under the directory opened in the IDE, but it's unclear what the UX may be for users with "too many" root modules.This may lead to high CPU and/or memory usage for potentially long time, before the user even opens a single file which is certainly not a great UX.
Proposal
We can find and set a high enough cap which won't affect most users, but still provide a decent UX for those who have many root modules.
The cap could be dynamic based on the amount of memory and/or CPU available, but we can't predict how much schema we obtain from plugins for each root module and so there should be a hard cap anyway.
We can collect some data and measure the performance with increasing amount of modules, but (completely uneducated guess) I would hope that we can reasonably support some hundreds of root modules at least.
Also/instead we can delay loading root modules until
didOpen
. This has a downside - obtaining schema may take some time and so completion and other features just won't work until the schema is obtained. This could be in reality low seconds per provider. Scheduling the schema loading could be configurable though.The text was updated successfully, but these errors were encountered: