I also want to add that if you explicitly set nsslapd-maxdescriptors that it will override the systemd default. So if you don't set nsslapd-maxdescriptors then the server will use the systemd FD limit (16384).

The fix looks good to me.
Should not it be the soft limit (rlim_cur). My understanding is that systemd sets the soft limit

The fix looks good to me.
Should not it be the soft limit (rlim_cur). My understanding is that systemd sets the soft limit

In my testing both max and cur were the same actually, but we "want" to set it to the maximum value (which sets the soft limit for DS):

[root@localhost 389-ds-base]# cat /proc/$(cat /var/run/dirsrv/slapd-localhost.pid)/limits  | grep -e Limit -e "open files"
Limit                     Soft Limit           Hard Limit           Units     
Max open files            16384                16384                files   

I thought hard limit is the system limit while soft limit is the process limit.
I would expect systemd to set the soft limit of the process. but it is just a guess.

I thought hard limit is the system limit while soft limit is the process limit.

Well the hard limit is the maximum value you can set the soft limit to. The process can only use what the soft limit is set to.

Previously the server just set it o 1024, now we check what the maximum allowed is, and we use that for the server's soft limit.

I would expect systemd to set the soft limit of the process. but it is just a guess.

It sets both soft and hard to 16384, but the server was overriding this with its own "low" default. Now we use what systemd uses.

I'm wondering why we attempt to translate and set a limit at all. We have an fd limit that actually just limits the size of the conntable.

@tbordaz There are multiple ways to limit FD's on linux .... linux resource management is a disaster to put it mildly. Systemd could be using cgroup, ulimit, or something else. Who ever knows!

This code should be with the other detector code in spal.c

I'm wondering why we attempt to translate and set a limit at all. We have an fd limit that actually just limits the size of the conntable.

@firstyear - I agree this setting is antiquated/obsolete/misleading. It should be called: nsslapd-max-concurrent-connections. But really it's a setting I don't think we need. Not sure what is the best way to move forward. Deprecate it in 1.4.1, and just ignore it 1.4.2? Or is there a good reason to limit the FDs the server uses?

This code should be with the other detector code in spal.c

I'm not finding a spal.c file in the source code...

Something I am missing.
The connection table is sized with the maximum file descriptor number.
But many other files will be opened by DS (libraries, logs, db_files...). So if we accept as much connections as set in maximum file descriptor number we should get an error.

Something I am missing.
The connection table is sized with the maximum file descriptor number.
But many other files will be opened by DS (libraries, logs, db_files...). So if we accept as much connections as set in maximum file descriptor number we should get an error.

We use the reserved-fd-limit (128) for these other files So the conn table gets: (max_fds - reserved_fds)

I'm wondering why we attempt to translate and set a limit at all. We have an fd limit that actually just limits the size of the conntable.

@firstyear - I agree this setting is antiquated/obsolete/misleading. It should be called: nsslapd-max-concurrent-connections. But really it's a setting I don't think we need. Not sure what is the best way to move forward. Deprecate it in 1.4.1, and just ignore it 1.4.2? Or is there a good reason to limit the FDs the server uses?

If we run out of FD's we'll just get an error case trying to open new resources, so the reason to have a cap is to limit connections so we don't run out of file handles for other ops. But more over instead of having a configuration parameter we should just detect this limit from the environment, and flag in log at startup "hey we picked this limit of connections, change this env variable if you want more / less".

EDIT: Which btw, if we run out of fd's due to client connections it could crash us, so CVE + DOS time ....

I'm wondering why we attempt to translate and set a limit at all. We have an fd limit that actually just limits the size of the conntable.
@firstyear - I agree this setting is antiquated/obsolete/misleading. It should be called: nsslapd-max-concurrent-connections. But really it's a setting I don't think we need. Not sure what is the best way to move forward. Deprecate it in 1.4.1, and just ignore it 1.4.2? Or is there a good reason to limit the FDs the server uses?

If we run out of FD's we'll just get an error case trying to open new resources, so the reason to have a cap is to limit connections so we don't run out of file handles for other ops. But more over instead of having a configuration parameter we should just detect this limit from the environment, and flag in log at startup "hey we picked this limit of connections, change this env variable if you want more / less".
EDIT: Which btw, if we run out of fd's due to client connections it could crash us, so CVE + DOS time ....

This is still something I have to fix in 1.3.9 for RHEL 7. I can not remove the configuration setting in RHEL 7, but I can add a log message saying what the server decided to use. Remember in this PR the new default comes from the environment.

Since I got an ack for this from @tbordaz I'm going to merge this, but I will leave the issue open so we can start looking into removing nsslapd-maxdescriptors in 1.4.x...

Which btw, if we run out of fd's due to client connections it could crash us, so CVE + DOS time

Well this is a problem no matter what we set for the FD limit - it can always be exhausted, right? :-p

Well, we can run out of connections which is fine, but we don't want to crash in that scenario, so having a reserve for internal server operations is useful, even if clients can't access us briefly.

So yep, let's look to remove this in the future :)

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/3380

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.