famedly / ansible-collection-base

`famedly.base` ansible collection for common basic services/tools like dns, ldap, ssh, ...
GNU Affero General Public License v3.0
0 stars 2 forks source link

Reload postgres config using pg_reload_conf #227

Closed jcgruenhage closed 2 months ago

jcgruenhage commented 4 months ago

Right now we're restarting postgres to reload the pg_hba, we should be using the pg_reload_conf function from https://www.postgresql.org/docs/current/functions-admin.html#FUNCTIONS-ADMIN-SIGNAL

blueorca363 commented 2 months ago

Seems like the handler task is the only part that must be edited: https://github.com/famedly/ansible-collection-base/blob/main/roles/postgresql/handlers/main.yml

https://www.heatware.net/postgresql/postgresql-reload-config-without-restarting/

blueorca363 commented 2 months ago

this only seems to work in some cases, though:

The configuration file is reread whenever the main server process receives a SIGHUP signal; this signal is most easily sent by running pg_ctl reload from the command line or by calling the SQL function pg_reload_conf(). The main server process also propagates this signal to all currently running server processes, so that existing sessions also adopt the new values (this will happen after they complete any currently-executing client command). Alternatively, you can send the signal to a single server process directly. Some parameters can only be set at server start; any changes to their entries in the configuration file will be ignored until the server is restarted. Invalid parameter settings in the configuration file are likewise ignored (but logged) during SIGHUP processing.

Source: https://www.postgresql.org/docs/current/config-setting.html

blueorca363 commented 2 months ago

As community.postgresql.postgresql_set uses the ALTER SYSTEM command, checking for a pending restart is quite easy via Ansible, and is already implemented. I simply added another task that always reloads the database in addition to restarting (which is only done when it is actually needed).

IMO it did not reload the database in the past, when no restart was required (even if a reload was required).

blueorca363 commented 2 months ago

I think this should work. However, I'd definitely prefer to test it somewhere before merging it.

EDIT: Did some changes, testing them on "dev-zeta".

blueorca363 commented 2 months ago

Ready for approval.