[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#8707) slapd: Add systemd service notification support



On Mon, Aug 07, 2017 at 01:30:44PM +0000, fumiyas@osstech.co.jp wrote:
>I've created patches for slapd to add systemd service notification 
>(sd_notify(3)) support.

Thanks for working on this! I'm also interested in having this feature.

What do you think about including a slapd.service file? I know OpenLDAP 
has traditionally not included an init script, but systemd units are 
intended to be distro-agnostic as far as possible, and shipped by 
upstream projects in most cases, unlike init scripts. Ideally I'd like 
to include a template in the source and have the build system fill in 
the autoconf'ed path (i.e. to slapd) and have 'make install' install it 
to the right place.

Autoconf bits look fine to me.

>+	rc = sd_notifyf( 1,
>+		"READY=1\n"
>+		"STATUS=slapd: ready to serve connections...\n"
>+		"MAINPID=%lu",
>+		(unsigned long) getpid() );

unset_environment=1 seems reasonable, it's a little unfortunate that we 
can't call (for example) sd_notify("STOPPING=1") afterward, but it feels 
worthwhile compared to having to sanitize the environment when forking a 
child process.

I'm not sure the STATUS= message adds value compared to just the basic 
readiness notification; can you comment on why you included it?

I guess MAINPID= is actually needed, unless we run slapd with -d, regardless of
whether we set Type=forking or Type=notify. Not exactly "needed", but better to
have it than not.

I see you've placed this call later than the parent's exit point. Any 
comments about the timing of this relative to the parent's exit, and to 
the listener startup and so on? Are the listeners more likely to be 
ready to serve connections at this point? I seem to recall that in the 
past there was opposition to moving the parent's exit later, but I can't 
remember why. (and I still wish we could do that, and dispense with the 
ldapsearch-loop hacks...)

cheers,
Ryan