-
Notifications
You must be signed in to change notification settings - Fork 197
CFE-3625: Increase Filedescriptor Limit to a more contemporary Value #4609
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
CFE-3625: Increase Filedescriptor Limit to a more contemporary Value #4609
Conversation
Ticket: CFE-3625 Changelog: Title
|
@cf-bottom please test this in our Jenkins CI system :) |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/6470/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-6470/ |
vpodzime
left a comment
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.
Looks good to me otherwise.
libpromises/pipes_unix.c
Outdated
|
|
||
| static pid_t *CHILDREN = NULL; /* GLOBAL_X */ | ||
| static int MAX_FD = 128; /* GLOBAL_X */ /* Max number of simultaneous pipes */ | ||
| static int MAX_FD = 8192; /* GLOBAL_X */ /* Max number of simultaneous pipes */ |
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.
Wow, big hammer approach. :) I'm not quite sure 8192 child processes (or even 4096) running at the same time is feasible. What about 2048?
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.
:) yeah, maybe a little bit overstated, i like to live dangerously. 2048 should be enough. i am trapped in AIX and big machine thinking.
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.
I was going to suggest 640k
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.
hmm...basically i can patch this locally, as i am constantly building cfengine rpms for aix. NorthernTechHQ/libntech#130 looks very "promising" to me, and i dont want to fire up a "numbers" flamewar :-). this problem seems to be a isolated thing (only me) anyway. nobody is doing stupid stuff like me g
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.
128 is definitely a very low and totally artificial limit. 2048 should be fine for everybody. 😉
|
Just a note: I'm currently working on NorthernTechHQ/libntech#130 which should be a better long-term solution for this and many other similar problems. Having said that, we should definitely make this simple change first. |
Co-authored-by: Ole Herman Schumacher Elgesem <olehermanse@users.noreply.github.com>
|
@flynn1973 could you please squash the commits? Thanks! |
No changelog entry since this memory is not really leaking, it's just not cleaned up properly right before an exit(). Changelog: None Ticket: CFE-3431 Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech>
Also added acceptance test. Changelog: unless can now be used with custom promise types Ticket: CFE-3431 Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech>
Ticket: CFE-3625 Changelog: Title
|
superseeded by #4615 |
Ticket: CFE-3625
Changelog: Title