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

Set reasonable default for MAX_FORKS #5674

Merged

Conversation

jakemcdermott
Copy link
Contributor

@jakemcdermott jakemcdermott commented Jan 15, 2020

SUMMARY

Set a reasonably sane default for MAX_FORKS.
#5142

Copy link
Contributor

@ryanpetrello ryanpetrello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good idea to help people avoid shooting themselves in the foot.

cc @matburt @wenottingham @tvo

awx/main/conf.py Outdated
register(
'MAX_FORKS',
field_class=fields.IntegerField,
allow_null=False,
default=0,
default=100,
Copy link
Contributor

@ryanpetrello ryanpetrello Jan 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I heard various numbers thrown around in our chat earlier w/ @sivel and @jctanner.

Do we know if we have any users setting this higher in general on hardware with lots of CPUs? Would 200 be on the safer side to avoid surprises for larger users?

Copy link
Contributor Author

@jakemcdermott jakemcdermott Jan 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? Bumping it up to 200 sounds fine for those types of users - Though if we take 25 forks per 4GB as our baseline it'd be good to know if 200 would prevent a bad time for all the folks just running our recommended 4GB minimum.

Based on our earlier chat, 100 was on the upper side of reasonable for a lot of scenarios. However, since this meant to be more of a sanity check I see your point that nudging this closer to 200 is probably the way to go. Some users might be doing alright with > 100 and so the additional warnings on upgrade wouldn't be welcome.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@jakemcdermott jakemcdermott changed the title Set default MAX_FORKS to 100 Set reasonable default for MAX_FORKS Jan 15, 2020
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit f911fb2 into ansible:devel Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants