Use system fs.max-files as limits instead of hard-coded value #142
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
This change is |
| maxConns, err := sysctl.New().GetSysctl("fs/file-max") | ||
| if err != nil { | ||
| glog.Errorf("unexpected error reading system maximum number of open file descriptors (fs.file-max): %v", err) | ||
| return 0 |
There was a problem hiding this comment.
Maybe a comment here that this means "don't render", not that the max will be 0 :-)
| return 0 | ||
| } | ||
|
|
||
| return maxConns |
There was a problem hiding this comment.
Should we have some "padding" e.g. maxConns - 1024 ? Also, is worker_rlimit_nofile per worker process . I honestly just don't know - I'm happy to follow your lead here if you say "this is correct", just not sure I follow the logic of letting a single nginx process use up all the FDs available to the whole system?
There was a problem hiding this comment.
I honestly just don't know - I'm happy to follow your lead here if you say "this is correct", just not sure I follow the logic of letting a single nginx process use up all the FDs available to the whole system?
Actually this process can use just the available FDs in the system.
If nginx tries to use more than the available the log just will show "Too many open files" which means:
- the current
fs.file-maxvalue is low - the node is small
- there is a process using/leaking descriptors
This setting tries to use the available resources (if needed) and not require scale the pod just because we are using the default values and we cannot get more than a couple of thousand connections.
|
@justinsb I think this is an improvement. We can review this after a release. |
|
OK, SGTM! |
fixes kubernetes-retired/contrib#1966