From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55107) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ese3t-0008Cn-JZ for guix-patches@gnu.org; Sun, 04 Mar 2018 19:33:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ese3q-0000U8-Re for guix-patches@gnu.org; Sun, 04 Mar 2018 19:33:05 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:36984) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ese3q-0000U0-My for guix-patches@gnu.org; Sun, 04 Mar 2018 19:33:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ese3q-0003Tv-F4 for guix-patches@gnu.org; Sun, 04 Mar 2018 19:33:02 -0500 Subject: [bug#30701] [PATCH 2/4] services: Use a external pid file for PostgreSQL. Resent-Message-ID: References: <20180304191633.20262-1-mail@cbaines.net> <20180304191633.20262-2-mail@cbaines.net> From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <20180304191633.20262-2-mail@cbaines.net> Date: Mon, 05 Mar 2018 01:32:18 +0100 Message-ID: <87k1urz7jh.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Christopher Baines Cc: 30701@debbugs.gnu.org Christopher Baines writes: > * gnu/services/databases.scm (postgresql-pid-file-for-version): New procedure. > (): 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 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 port locale config-file data-directory) > + (($ 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.