Skip to content
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

[BUG] apache.vhosts.standard (and potentially others) generate MemoryError #297

Open
ixs opened this issue Dec 8, 2020 · 9 comments
Open
Labels

Comments

@ixs
Copy link
Contributor

ixs commented Dec 8, 2020

I have a setup here that has about 200 vhosts using the apache.vhosts.standard state.

state.apply calls fail with a MemoryError and slsutil.renderer generates about 1GB of json...

This seems due to the full apache variable being passed multiple times as context thus exponentially increasing memory consumption.

apache: {{ apache|json }}

map: {{ apache|json }}

Is an example where the full apache variable is passed in twice...

@ixs ixs added the bug label Dec 8, 2020
@myii
Copy link
Member

myii commented Dec 8, 2020

Thanks for the report, @ixs. I add that the entries under the context block need to be indented by another 2 spaces as well:

- context:
apache: {{ apache|json }}
id: {{ id|json }}
site: {{ site|json }}
map: {{ apache|json }}

Meaning:

     - context: 
         apache: {{ apache|json }} 
         id: {{ id|json }} 
         ...

@noelmcloughlin Could you have a look at this, please?

@noelmcloughlin
Copy link
Member

noelmcloughlin commented Dec 8, 2020 via email

@ixs
Copy link
Contributor Author

ixs commented Dec 8, 2020

No need. I have something locally which I'm testing already as PR.

@myii
Copy link
Member

myii commented Dec 8, 2020

How did I become a code owner ;-) We need to encourage issue raisers to contribute PRs too. Okay, the value apache: {{ apache|json }} was added a few years ago, I can raise a PR.

@noelmcloughlin Well, I usually check the blame to see who last changed the section which has caused the regression and then ping accordingly. It's nothing personal! However, you're probably going to get more pings when there's wholescale changes made to formulas!

@noelmcloughlin
Copy link
Member

noelmcloughlin commented Dec 8, 2020 via email

@ixs
Copy link
Contributor Author

ixs commented Dec 8, 2020

#298 is the PR... Works fine locally. We're still passing way too much information per templating call but at least we're not using exponentially more memory...

@ixs
Copy link
Contributor Author

ixs commented Dec 8, 2020

PR merged. closing. Quick turnaround, thanks guys.

@noelmcloughlin
Copy link
Member

Thanks for the issue. Lets keep it open for a while.

I raised #299 as follow-up. There may be more but this PR was obvious.

@noelmcloughlin noelmcloughlin reopened this Dec 8, 2020
@ixs
Copy link
Contributor Author

ixs commented Dec 8, 2020

👍🏻

There are more cases where full dicts are passed,but the bad ones are where it's done per iteration like in vhosts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants