PR#50852: Issue 49990 - Need to enforce a hard maximum limit for file descriptors - 389-ds-base

389-ds-base

#50852 Issue 49990 - Need to enforce a hard maximum limit for file descriptors

Closed 3 years ago by spichugi. Opened 4 years ago by mreynolds.

mreynolds/389-ds-base issue49990 into master

Issue 49990 - Need to enforce a hard maximum limit for file descriptors

Mark Reynolds • 4 years ago

54b941d

ldap/servers/slapd/libglobs.c

file modified

+7 -3

		`@@ -1579,7 +1579,9 @@`
		`#endif`
		`/* Default the maximum fd's to the maximum allowed */`
		`if (getrlimit(RLIMIT_NOFILE, &rlp) == 0) {`
		`- maxdescriptors = (int64_t)rlp.rlim_max;`
		`+ if ((int64_t)rlp.rlim_max < SLAPD_DEFAULT_MAXDESCRIPTORS) {`
		`+ maxdescriptors = (int64_t)rlp.rlim_max;`
		`+ }`
		`}`

		`/* Take the lock to make sure we barrier correctly. */`
		`@@ -4355,7 +4357,7 @@`
		`{`
		`int32_t retVal = LDAP_SUCCESS;`
		`int64_t nValue = 0;`
		`- int64_t maxVal = 524288;`
		`+ int64_t maxVal = SLAPD_DEFAULT_MAXDESCRIPTORS;`
		`struct rlimit rlp;`
		`char *endp = NULL;`

		`@@ -4366,7 +4368,9 @@`
		`}`

		`if (0 == getrlimit(RLIMIT_NOFILE, &rlp)) {`
		`- maxVal = (int)rlp.rlim_max;`
		`+ if ((int64_t)rlp.rlim_max < maxVal) {`
		`+ maxVal = (int64_t)rlp.rlim_max;`
		`+ }`
		`}`

		`errno = 0;`

ldap/servers/slapd/slap.h

file modified

+2 -2

		`@@ -350,8 +350,8 @@`

		`#define SLAPD_DEFAULT_PAGEDSIZELIMIT 0`
		`#define SLAPD_DEFAULT_PAGEDSIZELIMIT_STR "0"`
		`- #define SLAPD_DEFAULT_MAXDESCRIPTORS 8192`
		`- #define SLAPD_DEFAULT_MAXDESCRIPTORS_STR "8192"`
		`+ #define SLAPD_DEFAULT_MAXDESCRIPTORS 1048576`
		`+ #define SLAPD_DEFAULT_MAXDESCRIPTORS_STR "1048576"`
		`#define SLAPD_DEFAULT_MAX_FILTER_NEST_LEVEL 40`
		`#define SLAPD_DEFAULT_MAX_FILTER_NEST_LEVEL_STR "40"`
		`#define SLAPD_DEFAULT_GROUPEVALNESTLEVEL 0`

mreynolds commented 4 years ago

Description:

on some platforms the maximum FD limit is high it can cause a OOM at server startup. So we need to add a hard maximum limit.

relates: https://pagure.io/389-ds-base/issue/49990

firstyear commented 4 years ago

looks fine to me - I think it will make the freelist patch more important though ....

firstyear commented 4 years ago

Ack btw

tbordaz commented 4 years ago

Fix looks good, just a question @mreynolds any reason to increase the default maxdescriptor of the daemon from 512K to 1M ? It looks to me that 512K is already high regarding C10K capability.

mreynolds commented 4 years ago

Fix looks good, just a question @mreynolds any reason to increase the default maxdescriptor of the daemon from 512K to 1M ? It looks to me that 512K is already high regarding C10K capability.

@vashirov suggested 1m as it's the kernel default. See the issue for his exact comment: https://pagure.io/389-ds-base/issue/49990

rebased onto 54b941d

4 years ago

Pull-Request has been merged by mreynolds

4 years ago

spichugi commented 3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3906

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 2