When should the Installer restart Puma

Ohai,

today, in production deployments, to reload the app (e.g. because the code changed), one needs to restart the foreman.service, which hosts our Puma service.

This is done by the installer in roughly to places:

To translate this from Puppet to English:

  • if the installer performed any basic install steps (like installing core packages) it will restart Puma
  • if the installer changed anything in the configuration it will restart Puma
  • if the installer did any database changes (like running migrations) it will restart Puma
  • if the installer installed a plugin it will restart Puma

While this sounds solid on the first look, one can construct cases where Puma is not restarted:

  • a core package is updated via the package manager, but has no new migrations (this is luckily handled by packaging, tho)
  • a plugin is updated via the package manager, but has no new migrations
  • a plugin is installed via the package manager, but has no (new) migrations

The last one is not that bad, as the worst case is that the new plugin is not loaded at all and doesn’t do any harm.

But the other (I ignore core, as that is handled by packaging) is a bit more involved. It results in on-disk and in-memory code being not in sync, and probably resulting in the user thinking a fix was applied, while it really was not. Another problem is that Ruby installs its gems in versioned folders (like katello-4.2.0), so when we update (e.g. to katello-4.2.1) and don’t restart Puma Rails could try to read templates (or other files) from disk in the 4.2.0 folder, which doesn’t exist anymore.

Thus, I think, we should see how we can restart Puma more often. At the same time, restarting it after every plugin installation/upgrade can result in way too many restarts when performing actions on multiple plugins (like when upgrading from one Foreman version to another).

So, thoughts and ideas welcome how to fix this :slight_smile:

1 Like

Two quick thoughts that come to mind:

  1. Add the ability on start up to compare the known set of plugins and versions with the “new” set being loaded, and have some task that is checking this? It could raise a notification to the user “new plugins available but the app needs restarting” similar to Jenkins or Chrome. This is not automatic but raises awareness.
  2. Have plugins touch a file on disk when they are updated that also gets touched when we restart Puma to allow detecting out of sync. Then use something (hopefully not a cronjob) to monitor and issue a restart. Perhaps using one of these techniques – https://github.com/puma/puma/blob/master/docs/restart.md

I think we are up against how much do we want to do automatically (and potentially lead to multiple restarts and downtime) with how much can we notify the user of changes so they can choose to take action. The latter might be impossible though with your template example.

How about writing a file to the Rails temp folder on plugin installation (I’ll let it undefined for now what “installation” means¹), and let the app show a warning if that file is present and non-empty? If we write plugin_name (plugin_version) for every newly installed plugin there, we even can make the warning contain this information.

On the other side, booting the Rails app would empty (or remove) that file. And the installer could also ensure absent (or empty) it, thus having a way to trigger a service restart?

Âą: for package installs, this is easy, we can just add a snippet to our post install scripts, for pure gem installs it is not, but then again, I am not too worried about this case to begin with.

This Fedora 34 change comes to mind:
https://fedoraproject.org/wiki/Changes/Restart_services_at_end_of_rpm_transaction

Essentially what it does is call /usr/bin/systemctl set-property $unit Markers=+needs-restart in %post (which is cheap). Then it has a trigger that calls systemctl reload-or-restart --marked.

If then needs migrations, it will stop serving requests (this is built into Foreman). The installer will then perform those migrations and restart the service.

So if we use %systemd_post on EL9, we should benefit from this. Sadly, we can’t rely on that just yet.

So for now I agree with the “touch a file somewhere and have the installer ensure it’s absent (which would trigger a service restart)”.

I’ve opened Bug #34602: detect plugin installation and trigger puma restart - Installer - Foreman to further tackle this.