@mreynolds, my understanding is that using using NS the shutdown signal handler is in the hand of NS. Without NS a signal calls g_set_shutdown with with NS it calls ns_set_shutdown that calls g_set_shutdown. So regarding signals in both cases the server is aware it should shutdown.

Is the ticket about direct calls to g_set_shutdown when the server wants to shutdown (disk space or network listen failure or log failure) ?

@mreynolds, my understanding is that using using NS the shutdown signal handler is in the hand of NS. Without NS a signal calls g_set_shutdown with with NS it calls ns_set_shutdown that calls g_set_shutdown. So regarding signals in both cases the server is aware it should shutdown.
Is the ticket about direct calls to g_set_shutdown when the server wants to shutdown (disk space or network listen failure or log failure) ?

Yes, and I can promise you just setting g_set_shudown() does not stop the slapd-daemon. It will stop other "loops" in the code, but Nunc-stans just waits forever until its threads get a signal.

I agree that g_set_shutdown() does not affect NS. If we use NS we should schedule a NS shutdown (ns_set_shutdown) and possibly wait for its completion.
Regarding raise(SIGTERM) I am unsure what is you proposal. I think signal handling works well enough and it could be simpler option to replace g_set_shutdown (for direct call) with ns_set_shutdown.

I agree that g_set_shutdown() does not affect NS. If we use NS we should schedule a NS shutdown (ns_set_shutdown) and possibly wait for its completion.
Regarding raise(SIGTERM) I am unsure what is you proposal. I think signal handling works well enough and it could be simpler option to replace g_set_shutdown (for direct call) with ns_set_shutdown.

We don't know the job struct to pass to ns_set_shutdown() from g_set_shutdown(). In my testing I just added a raise(SIGTERM) at the end of g_set_shutdown(), and that forces the server to "cleanly" shutdown. Basically the server thinks someone ran "stop-dirsrv" - it does not trigger an exit() if that's what you were worried about.

Okay so g_set_shutdown will be called by NS signal handler. That means any reason it shuts down will be overwritten with SLAPI_SHUTDOWN_SIGNAL. Note there is a couple of places where shutdown for SLAPI_SHUTDOWN_DISKFULL reason is specifically tested.

void
g_set_shutdown(int reason)
{
    slapd_shutdown = reason;
    raise(SIGTERM);
}

slapd_shutdown does maintain its value through to the end (see ns_set_shutdown() - we only change the signal if its NOT already set), so those checks for DISK_FULL are still valid. Am I missing something (probably)? :-)

Okay I see the issue, its actually in set_shutdown() where it gets overwritten. I'm going to apply the same logic to this function as what is in ns_set_shutdown().

And really I am fixing this as part of a Disk Monitoring fix. So I will actually close this out and use this ticket to implement the change: https://pagure.io/389-ds-base/issue/49734

Oky, so I missed this.

The way that NS works, it's NOT spinning on and checking an atomic all the time. This is a HUGE bottle neck! The way that a shut down is triggered is via a signal (as you have done, but a bit messy), or to actually call the ns shutdown function - they have the same effect.

There are great reasons for this design, mainly around trying to unblock thread contention on workers. In fact this whole "atomic for are we running" will be removed in the future, because if a worker is alive, we are running - there is no ambiguous state. Every job runs to completion.

This has been addressed, closing

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2811

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.