[bug#30701] [PATCH 2/4] services: Use a external pid file for PostgreSQL.

[Clément Lassieur]

Christopher Baines <address@hidden> writes:

> * gnu/services/databases.scm (postgresql-pid-file-for-version): New procedure.
>   (<postgresql-config-file>): Add a new external-pid-file field.
>   (postgresql-config-file-compiler): Add support for the external-pid-file.
>   (postgresql-activation): Create the directory for the pid file.
>   (postgresql-shepherd-service): Use the pid-file when starting the service.
> ---
>  gnu/services/databases.scm | 48 
> ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 36 insertions(+), 12 deletions(-)

...

> +(define (postgresql-pid-file-for-version version)
> +  (string-append "/var/run/postgresql/"
> +                 (version-major+minor version)
> +                 "-main.pid"))
> +
>  (define-record-type* <postgresql-config-file>
>    postgresql-config-file make-postgresql-config-file
>    postgresql-config-file?
> -  (log-destination postgresql-config-file-log-destination
> -                   (default "syslog"))
> -  (hba-file        postgresql-config-file-hba-file
> -                   (default %default-postgres-hba))
> -  (ident-file      postgresql-config-file-ident-file
> -                   (default %default-postgres-ident))
> -  (extra-config    postgresql-config-file-extra-config
> -                   (default '())))
> +  (log-destination   postgresql-config-file-log-destination
> +                     (default "syslog"))
> +  (hba-file          postgresql-config-file-hba-file
> +                     (default %default-postgres-hba))
> +  (ident-file        postgresql-config-file-ident-file
> +                     (default %default-postgres-ident))
> +  (external-pid-file postgresql-config-file-external-pid-file
> +                     (default (postgresql-pid-file-for-version
> +                               (package-version postgresql))))

Why does external-pid-file have a default value.  It's optional, and the
user already has access to $DATA/postmaster.pid anyway.

> @@ -140,6 +153,9 @@ host      all     all     ::1/128         trust"))
>                    (default 5432))
>    (locale         postgresql-configuration-locale
>                    (default "en_US.utf8"))
> +  (pid-file       postgresql-configuration-pid-file
> +                  (default (postgresql-pid-file-for-version
> +                               (package-version postgresql))))

The main PID file is chosen by Postgres, and put at
$DATA/postmaster.pid.  I don't think it's customizable.  This setting
(pid-file) probably doesn't affect the daemon the way you think it does.

>    (config-file    postgresql-configuration-file
>                    (default (postgresql-config-file)))
>    (data-directory postgresql-configuration-data-directory
> @@ -157,7 +173,8 @@ host      all     all     ::1/128         trust"))
>  
> +           ;; Create a directory for the pid file
> +           (mkdir-p #$(dirname pid-file))
> +           (chown #$(dirname pid-file) (passwd:uid user) (passwd:gid user))

I think it would make more sense to create the directory for the
external-pid-file.

>  (define postgresql-shepherd-service
>    (match-lambda
> -    (($ <postgresql-configuration> postgresql port locale config-file 
> data-directory)
> +    (($ <postgresql-configuration> postgresql port locale pid-file
> +                                   config-file data-directory)
>       (let* ((pg_ctl-wrapper
>               ;; Wrapper script that switches to the 'postgres' user before
>               ;; launching daemon.
> @@ -221,7 +243,9 @@ host      all     all     ::1/128         trust"))
>                (provision '(postgres))
>                (documentation "Run the PostgreSQL daemon.")
>                (requirement '(user-processes loopback syslogd))
> -              (start (action "start"))
> +              (start #~(make-forkexec-constructor
> +                        '(#$pg_ctl-wrapper "start")
> +                        #:pid-file #$pid-file))
>                (stop (action "stop"))))))))

If pid-file != external-pid-file, Sherpherd will wait for a file that
doesn't exist won't it?  Because Postgresql will never be aware of that
pid-file.

Plus, there is no reason to use make-forkexec-constructor on pg_ctl
because pg_ctl returns after it has checked that the daemon is running.
For the same reason, Shepherd doesn't need to know about Postgres' PID.
All the hard work is done by pg_ctl.