UCarp icon indicating copy to clipboard operation
UCarp copied to clipboard

Pidfile redux

Open jsumners opened this issue 10 years ago • 14 comments

As with pull request #16, this pull request adds support for a pidfile except it fixes the issue where the pidfile was not removed on shutdown. With this pull request if ucarp is invoked with --pidfile and --shutdown then the pidfile will unlinked on termination.

jsumners avatar Nov 10 '15 20:11 jsumners

Okay, I switched to simply using fprintf. At the same time, I added in a LOG_ERR if the destination pid file could not be opened.

jsumners avatar Nov 20 '15 18:11 jsumners

Hey @jsumners. Thanks for submitting this PR and improving it. There have been a few submissions for creating a pid-file directly from the program, so this feature is obviously an important one. @jedisct1 has been too busy to give this legacy project much attention, so he granted me commit privileges on this repo. In the next few days, I plan to pull in some new features and come up with a new point release to push downstream. While I appreciate your effort on this feature, @miconda has a similar approach at https://github.com/miconda/UCarp/commit/109a7950de550f1354caf24e65da383659395e22, and his experience coding in C shows. Thanks for your input and effort.

petiepooo avatar Nov 06 '17 23:11 petiepooo

😞 It would be nice if the other PR used the same shorthand switch (-Z). I have my branch in production -- https://github.com/jsumners/ucarp-rhel7 -- but want to use upstream.

jsumners avatar Nov 07 '17 13:11 jsumners

To give my opinion: if the fist letter of the switch is already used in both lower and upper case, then I tend to select one that is not yet use at all (i.e., none of the lower or upper case). -z is used for shutdown script, so was ruled out from my point of view. The reason behind is that I like to have the option available in case a new switch which starts with that letter will be added in the future. Just personal preference, overall I am fine with any letter that is decided to be used.

miconda avatar Nov 07 '17 17:11 miconda

I went with -Z because -z is already used and the two are related (process management). But it's not really that big of a deal. I'll just have to remember to update my Ansible project to re-deploy my scripts with the new switch.

jsumners avatar Nov 07 '17 21:11 jsumners

I have no preference of one over the other. Unfortunately, one of you will be disappointed in either case, as it sounds you're both using your branch in production. I can't help that..

@jsumners, I understand writing pid-files is an important feature to many, but I didn't need that ability when using upstream's version in Ubuntu 16.04. Why is it important to your application?

petiepooo avatar Nov 09 '17 17:11 petiepooo

More like it's important to systemd so that it can track the process correctly. And re-reviewing my ucarp RPM, it looks like I am using the long form --pidfile. So, ignore my gripe.

jsumners avatar Nov 09 '17 17:11 jsumners

Ah, ok. I've been using it on Ubuntu systems, and it's launched from the helper script at /etc/network/if-up.d/ucarp, not directly from systemd. I'll take a gander at the RPM and see how Fedora/RedHat does it. Thanks.

petiepooo avatar Nov 09 '17 18:11 petiepooo

It's only available in the EPEL repository, and they packaged it a bit wrong. Look at mine instead: https://github.com/jsumners/ucarp-rhel7

Specifically, https://github.com/jsumners/ucarp-rhel7/blob/d29991855f79b90cd50546fa0874cf052f270f4d/files/ucarp is what is launched to start the processes.

jsumners avatar Nov 09 '17 18:11 jsumners

You have Type=forking commented out in your service file there, which means it defaults to Type=simple, and there is no need for a pidfile at all. With Type=simple, systemd runs the process in the foreground and handles daemonization and pid tracking natively.

From https://www.freedesktop.org/software/systemd/man/systemd.service.html:

Type= Configures the process start-up type for this service unit. One of simple, forking, oneshot, dbus, notify or idle.

If set to simple (the default if neither Type= nor BusName=, but ExecStart= are specified), it is expected that the process configured with ExecStart= is the main process of the service. In this mode, if the process offers functionality to other processes on the system, its communication channels should be installed before the daemon is started up (e.g. sockets set up by systemd, via socket activation), as systemd will immediately proceed starting follow-up units.

If set to forking, it is expected that the process configured with ExecStart= will call fork() as part of its start-up. The parent process is expected to exit when start-up is complete and all communication channels are set up. The child continues to run as the main daemon process. This is the behavior of traditional UNIX daemons. If this setting is used, it is recommended to also use the PIDFile= option, so that systemd can identify the main process of the daemon. systemd will proceed with starting follow-up units as soon as the parent process exits.

and

PIDFile= Takes an absolute filename pointing to the PID file of this daemon. Use of this option is recommended for services where Type= is set to forking. systemd will read the PID of the main process of the daemon after start-up of the service. systemd will not write to the file configured here, although it will remove the file after the service has shut down if it still exists.

The only reason I can see for using Type=forking here is if there are dependent services. Traditional svsvinit daemons signaled their ready-state by forking. Systemd can use that to detect a failed startup (eg. cannot bind, or expected file missing). With simple service types, systemd considers the service successfully launched as soon as the exec completes.

I don't think that would make a difference with ucarp. The appearance of the VIP is not a sign of successful service launching; it's a sign of the service becoming MASTER. You'd likely want any dependent services to start regardless of the ucarp state on startup. You'd want to know ucarp failed to start, of course, but it's not a reason to stop the boot process, and issues could likely be resolved without needing a reboot.

That said, the EPEL RPM apparently does use Type=forking, and they must have systemd guess the resulting daemon's pid. What is your objection to their packaging, if you can remember?

petiepooo avatar Nov 10 '17 16:11 petiepooo

To add to that:

GuessMainPID= Takes a boolean value that specifies whether systemd should try to guess the main PID of a service if it cannot be determined reliably. This option is ignored unless Type=forking is set and PIDFile= is unset because for the other types or with an explicitly configured PID file, the main PID is always known. The guessing algorithm might come to incorrect conclusions if a daemon consists of more than one process. If the main PID cannot be determined, failure detection and automatic restarting of a service will not work reliably. Defaults to yes.

Since ucarp is a single process, it should be able to guess the PID correctly if the type is set to forking, even if no pidfile is specified. I was under the impression, with systemd's use of cgroups, it could follow a process from startup to daemonization even if there are multiple instances launched at the same time, being that those processes are launched in different cgroups.

@miconda, do you have any insight into this? Why are you finding the need to write a pidfile on startup?

Reopening this for now while we hash this out...

petiepooo avatar Nov 10 '17 17:11 petiepooo

You have Type=forking commented out in your service file there, which means it defaults to Type=simple, and there is no need for a pidfile at all. With Type=simple, systemd runs the process in the foreground and handles daemonization and pid tracking natively.

I'm not claiming to know all of the bullshit involved with systemd. All I know is it wasn't working with Type=forking. At least not when using the [email protected] mechanism so that I can have many instances of ucarp tied to a single ucarp.service. And being able to generate a pidfile makes it easy to deal with the situation.

That said, the EPEL RPM apparently does use Type=forking, and they must have systemd guess the resulting daemon's pid. What is your objection to their packaging, if you can remember?

Their launch script is still the same as it was for SysV init. It is not really compatible with systemd. See https://src.fedoraproject.org/cgit/rpms/ucarp.git/tree/ucarp

jsumners avatar Nov 10 '17 17:11 jsumners

Yeah, it looks like the guy who put together the RPM for EPEL didn't put much work into making it compliant with systemd and making sure it works as as intended. FWIW, I'm not a big systemd fan myself, bit it's what's been chosen by most distros now..

petiepooo avatar Nov 10 '17 21:11 petiepooo

Following up with a bit delay, being caught by some other tasks lately ...

Even systemd or other start-stop init.d scripts can follow a daemon app by pid and write the pid file themselves, there are so many distros out there and startup systems, many not having such capability. Moreover, in some cases I have to use ucarp in rather old systems, released when no systemd was out.

Anyhow, my main need is to be able to use ucarp directly with monit (https://mmonit.com/monit/), a monitoring tool available in all distros I had to deal with. This tool has the capability to do more than simple process table watch for many apps (like mysql server, apache, ...), so usually I let the OS to start monit on startup and then monit start and supervise the critical apps there.

Writing a pid file is a common feature of daemons out there, is not hard to implement and makes possible to supervise ucarp on all systems, with or without systemd.

miconda avatar Nov 17 '17 19:11 miconda