bug#29992: 'postgres' service doesn't stop

bug#29992: 'postgres' service doesn't stop

[Clément Lassieur]

1. add (postgresql-service) to the service list
2. reconfigure
3. ps -ef | grep postgres

--8<---------------cut here---------------start------------->8---
postgres 19058     1  0 16:21 ?        00:00:00 /gnu/store/0v539yjmdqhjm1xcpvndmagkgjz5fvh2-guile-2.2.2/bin/guile --no-auto-compile /gnu/store/rp6qyfv5wyznz5a1i6szzzy997v603sl-start-postgres                                             
postgres 19061 19058  0 16:21 ?        00:00:00 /gnu/store/3rg0avphlwgq56qjxd8gr8xbmh1rwkal-postgresql-10.1/bin/postgres --config-file=/gnu/store/7m8yrv4avricqkn92737b5mamlvaid34-postgresql.conf -p 5432 -D /var/lib/postgresql/data     
postgres 19063 19061  0 16:21 ?        00:00:00 postgres: checkpointer process
postgres 19064 19061  0 16:21 ?        00:00:00 postgres: writer process
postgres 19065 19061  0 16:21 ?        00:00:00 postgres: wal writer process
postgres 19066 19061  0 16:21 ?        00:00:00 postgres: autovacuum launcher process
postgres 19067 19061  0 16:21 ?        00:00:00 postgres: stats collector process
postgres 19068 19061  0 16:21 ?        00:00:00 postgres: bgworker: logical replication launcher
clement  19070 18802  0 16:21 pts/4    00:00:00 grep postgres
--8<---------------cut here---------------end--------------->8---

4. herd stop postgres (output: Service postgres has been stopped)
5. ps -ef | grep postgres

--8<---------------cut here---------------start------------->8---
postgres 19061     1  0 16:21 ?        00:00:00 /gnu/store/3rg0avphlwgq56qjxd8gr8xbmh1rwkal-postgresql-10.1/bin/postgres --config-file=/gnu/store/7m8yrv4avricqkn92737b5mamlvaid34-postgresql.conf -p 5432 -D /var/lib/postgresql/data     
postgres 19063 19061  0 16:21 ?        00:00:00 postgres: checkpointer process
postgres 19064 19061  0 16:21 ?        00:00:00 postgres: writer process
postgres 19065 19061  0 16:21 ?        00:00:00 postgres: wal writer process
postgres 19066 19061  0 16:21 ?        00:00:00 postgres: autovacuum launcher process                                 
postgres 19067 19061  0 16:21 ?        00:00:00 postgres: stats collector process
postgres 19068 19061  0 16:21 ?        00:00:00 postgres: bgworker: logical replication launcher
clement  19078 18802  0 16:21 pts/4    00:00:00 grep postgres
--8<---------------cut here---------------end--------------->8---

bug#29992: 'postgres' service doesn't stop

[Ludovic Courtès]

Hi,

Clément Lassieur <clement@lassieur.org> skribis:

> 1. add (postgresql-service) to the service list
> 2. reconfigure
> 3. ps -ef | grep postgres
>
> postgres 19058     1  0 16:21 ?        00:00:00 /gnu/store/0v539yjmdqhjm1xcpvndmagkgjz5fvh2-guile-2.2.2/bin/guile --no-auto-compile /gnu/store/rp6qyfv5wyznz5a1i6szzzy997v603sl-start-postgres                                             
> postgres 19061 19058  0 16:21 ?        00:00:00 /gnu/store/3rg0avphlwgq56qjxd8gr8xbmh1rwkal-postgresql-10.1/bin/postgres --config-file=/gnu/store/7m8yrv4avricqkn92737b5mamlvaid34-postgresql.conf -p 5432 -D /var/lib/postgresql/data     
> postgres 19063 19061  0 16:21 ?        00:00:00 postgres: checkpointer process
> postgres 19064 19061  0 16:21 ?        00:00:00 postgres: writer process
> postgres 19065 19061  0 16:21 ?        00:00:00 postgres: wal writer process
> postgres 19066 19061  0 16:21 ?        00:00:00 postgres: autovacuum launcher process
> postgres 19067 19061  0 16:21 ?        00:00:00 postgres: stats collector process
> postgres 19068 19061  0 16:21 ?        00:00:00 postgres: bgworker: logical replication launcher
> clement  19070 18802  0 16:21 pts/4    00:00:00 grep postgres
>
>
> 4. herd stop postgres (output: Service postgres has been stopped)
> 5. ps -ef | grep postgres
>
> postgres 19061     1  0 16:21 ?        00:00:00 /gnu/store/3rg0avphlwgq56qjxd8gr8xbmh1rwkal-postgresql-10.1/bin/postgres --config-file=/gnu/store/7m8yrv4avricqkn92737b5mamlvaid34-postgresql.conf -p 5432 -D /var/lib/postgresql/data     
> postgres 19063 19061  0 16:21 ?        00:00:00 postgres: checkpointer process
> postgres 19064 19061  0 16:21 ?        00:00:00 postgres: writer process
> postgres 19065 19061  0 16:21 ?        00:00:00 postgres: wal writer process
> postgres 19066 19061  0 16:21 ?        00:00:00 postgres: autovacuum launcher process                                 
> postgres 19067 19061  0 16:21 ?        00:00:00 postgres: stats collector process
> postgres 19068 19061  0 16:21 ?        00:00:00 postgres: bgworker: logical replication launcher
> clement  19078 18802  0 16:21 pts/4    00:00:00 grep postgres

Perhaps the proper way to stop postgresql is via one of its client
commands, like we do for nginx?

HTH,
Ludo’.

bug#29992: 'postgres' service doesn't stop

[Clément Lassieur]

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> 1. add (postgresql-service) to the service list
>> 2. reconfigure
>> 3. ps -ef | grep postgres
>>
>> postgres 19058     1  0 16:21 ?        00:00:00 /gnu/store/0v539yjmdqhjm1xcpvndmagkgjz5fvh2-guile-2.2.2/bin/guile --no-auto-compile /gnu/store/rp6qyfv5wyznz5a1i6szzzy997v603sl-start-postgres                                             
>> postgres 19061 19058  0 16:21 ?        00:00:00 /gnu/store/3rg0avphlwgq56qjxd8gr8xbmh1rwkal-postgresql-10.1/bin/postgres --config-file=/gnu/store/7m8yrv4avricqkn92737b5mamlvaid34-postgresql.conf -p 5432 -D /var/lib/postgresql/data     
>> postgres 19063 19061  0 16:21 ?        00:00:00 postgres: checkpointer process
>> postgres 19064 19061  0 16:21 ?        00:00:00 postgres: writer process
>> postgres 19065 19061  0 16:21 ?        00:00:00 postgres: wal writer process
>> postgres 19066 19061  0 16:21 ?        00:00:00 postgres: autovacuum launcher process
>> postgres 19067 19061  0 16:21 ?        00:00:00 postgres: stats collector process
>> postgres 19068 19061  0 16:21 ?        00:00:00 postgres: bgworker: logical replication launcher
>> clement  19070 18802  0 16:21 pts/4    00:00:00 grep postgres
>>
>>
>> 4. herd stop postgres (output: Service postgres has been stopped)
>> 5. ps -ef | grep postgres
>>
>> postgres 19061     1  0 16:21 ?        00:00:00 /gnu/store/3rg0avphlwgq56qjxd8gr8xbmh1rwkal-postgresql-10.1/bin/postgres --config-file=/gnu/store/7m8yrv4avricqkn92737b5mamlvaid34-postgresql.conf -p 5432 -D /var/lib/postgresql/data     
>> postgres 19063 19061  0 16:21 ?        00:00:00 postgres: checkpointer process
>> postgres 19064 19061  0 16:21 ?        00:00:00 postgres: writer process
>> postgres 19065 19061  0 16:21 ?        00:00:00 postgres: wal writer process
>> postgres 19066 19061  0 16:21 ?        00:00:00 postgres: autovacuum launcher process                                 
>> postgres 19067 19061  0 16:21 ?        00:00:00 postgres: stats collector process
>> postgres 19068 19061  0 16:21 ?        00:00:00 postgres: bgworker: logical replication launcher
>> clement  19078 18802  0 16:21 pts/4    00:00:00 grep postgres
>
> Perhaps the proper way to stop postgresql is via one of its client
> commands, like we do for nginx?

Hi Ludovic,

Yes, probably.  I'll fix this when I have some time this week.

Clément

bug#29992: 'postgres' service doesn't stop

[Alex Kost]

Ludovic Courtès (2018-01-08 11:59 +0100) wrote:

> Perhaps the proper way to stop postgresql is via one of its client
> commands, like we do for nginx?

Regarding the commands: "pg_ctl" is the canonical tool to start/stop
PostgreSQL server.

Note: I don't use the Guix service for postgresql (instead I run
postgres via my Shepherd user instance).

-- 
Alex

bug#29992: 'postgres' service doesn't stop

[Catonano]

[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]

2018-01-08 22:55 GMT+01:00 Alex Kost <alezost@gmail.com>:

> Ludovic Courtès (2018-01-08 11:59 +0100) wrote:
>
> > Perhaps the proper way to stop postgresql is via one of its client
> > commands, like we do for nginx?
>
> Regarding the commands: "pg_ctl" is the canonical tool to start/stop
> PostgreSQL server.
>
> Note: I don't use the Guix service for postgresql (instead I run
> postgres via my Shepherd user instance).
>
> --
> Alex
>

I'm sorry if this isn't the place to write about this

My Tryton service doesn't get stopped either

Here's a picture I took from a virtual machine
https://imgur.com/a/LJ2Cq

When I run Tryton myself, I use this line:
trytond -c trytond.conf

this command doesn' t return a prompt
So, in order to stop Tryton I simply press Ctrl-C

In the definition of my sevice, I use
#~(make-kill-destructor)

to stop the Tryton service

But I copied this from other services, I don't know what
"make-kill-destructor" does

My Trytond service is not working anyway, I launch a vm with my service
running in it, I test  it with the client, I see it doesn't work, I stop it
and then I'd like to launch it by hand to see something more

But then, because a tryton-real daemon is still running, the socket for
that address is already bound, as you can see in the picture

Of course this has nothing to do with the Postgresql not stopping bug, I
was only giving some context

[-- Attachment #2: Type: text/html, Size: 2378 bytes --]

bug#29992: 'postgres' service doesn't stop

[Danny Milosavljevic]

Hi Catonano,

> When I run Tryton myself, I use this line:
> trytond -c trytond.conf
> this command doesn' t return a prompt

Does it stay in foreground with the tryton service, too?  It shouldn't do that for the service, see below.

make-forkexec-constructor should takes a pid-file argument which is supposed to refer to a file that trytond creates.  If that isn't specified, shepherd will default to the pid of the child process of shepherd that did the exec().  So it shouldn't have tryton in foreground in any case.

Note: make-forkexec-constructor also has a #:log-file keyword argument - which is handy for debugging.

> So, in order to stop Tryton I simply press Ctrl-C
> 
> In the definition of my sevice, I use
> #~(make-kill-destructor)
> 
> to stop the Tryton service
> 
> But I copied this from other services, I don't know what
> "make-kill-destructor" does

(define make-kill-destructor
  (lambda* (#:optional (signal SIGTERM))
    (lambda (pid . args)
      (kill pid signal)
      #f)))

So it sends SIGTERM to the process.

What happens when you send SIGTERM manually ("kill -TERM ...")?

> My Trytond service is not working anyway, I launch a vm with my service
> running in it, I test  it with the client, I see it doesn't work, I stop it
> and then I'd like to launch it by hand to see something more

You can try finding its pid with "ps -ef |grep tryton" and then compare their /proc/<pid> trees (manually started vs. service-started).

In your screenshot, that would be /proc/411, especially /proc/411/fd and its signal mask (in /proc/411/status).

> But then, because a tryton-real daemon is still running, the socket for
> that address is already bound, as you can see in the picture

That's good then.

Try "netstat -lpn | grep 411" (or whatever the pid is now) to find out whether it's actually listening.

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Clément Lassieur]

Fixes <https://bugs.gnu.org/29992>.

* gnu/services/databases.scm (postgresql-shepherd-service): Replace
make-forkexec-constructor and make-kill-destructor with pg_ctl.
---
 gnu/services/databases.scm | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index 6a01cb1ce..42f2315d5 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -147,26 +147,33 @@ host	all	all	::1/128 	trust"))
 (define postgresql-shepherd-service
   (match-lambda
     (($ <postgresql-configuration> postgresql port locale config-file data-directory)
-     (let ((start-script
-            ;; Wrapper script that switches to the 'postgres' user before
-            ;; launching daemon.
-            (program-file "start-postgres"
-                          #~(let ((user (getpwnam "postgres"))
-                                  (postgres (string-append #$postgresql
-                                                           "/bin/postgres")))
-                              (setgid (passwd:gid user))
-                              (setuid (passwd:uid user))
-                              (system* postgres
-                                       (string-append "--config-file="
-                                                      #$config-file)
-                                       "-p" (number->string #$port)
-                                       "-D" #$data-directory)))))
+     (let* ((pg_ctl-wrapper
+             ;; Wrapper script that switches to the 'postgres' user before
+             ;; launching daemon.
+             (program-file
+              "pg_ctl-wrapper"
+              #~(begin
+                  (use-modules (ice-9 match)
+                               (ice-9 format))
+                  (match (command-line)
+                    ((_ mode)
+                     (let ((user (getpwnam "postgres"))
+                           (pg_ctl #$(file-append postgresql "/bin/pg_ctl")))
+                       (setgid (passwd:gid user))
+                       (setuid (passwd:uid user))
+                       (system
+                        (format #f "~a -D ~a -o '--config-file=~a -p ~d' ~a"
+                                pg_ctl #$data-directory #$config-file #$port
+                                mode))))))))
+            (action (lambda args
+                      #~(lambda _
+                          (invoke #$pg_ctl-wrapper #$@args)))))
        (list (shepherd-service
               (provision '(postgres))
               (documentation "Run the PostgreSQL daemon.")
               (requirement '(user-processes loopback syslogd))
-              (start #~(make-forkexec-constructor #$start-script))
-              (stop #~(make-kill-destructor))))))))
+              (start (action "start"))
+              (stop (action "stop"))))))))
 
 (define postgresql-service-type
   (service-type (name 'postgresql)
-- 
2.16.0

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Ludovic Courtès]

Clément Lassieur <clement@lassieur.org> skribis:

> Fixes <https://bugs.gnu.org/29992>.
>
> * gnu/services/databases.scm (postgresql-shepherd-service): Replace
> make-forkexec-constructor and make-kill-destructor with pg_ctl.

[...]

> +     (let* ((pg_ctl-wrapper
> +             ;; Wrapper script that switches to the 'postgres' user before
> +             ;; launching daemon.
> +             (program-file
> +              "pg_ctl-wrapper"
> +              #~(begin
> +                  (use-modules (ice-9 match)
> +                               (ice-9 format))
> +                  (match (command-line)
> +                    ((_ mode)
> +                     (let ((user (getpwnam "postgres"))
> +                           (pg_ctl #$(file-append postgresql "/bin/pg_ctl")))
> +                       (setgid (passwd:gid user))
> +                       (setuid (passwd:uid user))
> +                       (system
> +                        (format #f "~a -D ~a -o '--config-file=~a -p ~d' ~a"
> +                                pg_ctl #$data-directory #$config-file #$port
> +                                mode))))))))

I think we should use ‘execl’ here instead of ‘system’ so that (1) the
exit code is correct, and (2) we don’t go through /bin/sh.

Apart from that it LGTM, thank you!

Ludo’.

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Clément Lassieur]

Ludovic Courtès <ludo@gnu.org> writes:

> Clément Lassieur <clement@lassieur.org> skribis:
>
>> Fixes <https://bugs.gnu.org/29992>.
>>
>> * gnu/services/databases.scm (postgresql-shepherd-service): Replace
>> make-forkexec-constructor and make-kill-destructor with pg_ctl.
>
> [...]
>
>> +     (let* ((pg_ctl-wrapper
>> +             ;; Wrapper script that switches to the 'postgres' user before
>> +             ;; launching daemon.
>> +             (program-file
>> +              "pg_ctl-wrapper"
>> +              #~(begin
>> +                  (use-modules (ice-9 match)
>> +                               (ice-9 format))
>> +                  (match (command-line)
>> +                    ((_ mode)
>> +                     (let ((user (getpwnam "postgres"))
>> +                           (pg_ctl #$(file-append postgresql "/bin/pg_ctl")))
>> +                       (setgid (passwd:gid user))
>> +                       (setuid (passwd:uid user))
>> +                       (system
>> +                        (format #f "~a -D ~a -o '--config-file=~a -p ~d' ~a"
>> +                                pg_ctl #$data-directory #$config-file #$port
>> +                                mode))))))))
>
> I think we should use ‘execl’ here instead of ‘system’ so that (1) the
> exit code is correct, and (2) we don’t go through /bin/sh.

Hi Ludovic, thank you for the review.

How do you pass single quotes as an ‘execl’ argument?

https://www.postgresql.org/docs/9.3/static/app-pg-ctl.html says:
--8<---------------cut here---------------start------------->8---
-o options
    Specifies options to be passed directly to the postgres command.
    The options should usually be surrounded by single or double quotes
    to ensure that they are passed through as a group.
--8<---------------cut here---------------end--------------->8---

Also, I don't understand how the exit code matters since Guile scripts
seem to always return 0, no matter if the last return value is true or
false.

Clément

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Clément Lassieur]

Clément Lassieur <clement@lassieur.org> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Clément Lassieur <clement@lassieur.org> skribis:
>>
>>> Fixes <https://bugs.gnu.org/29992>.
>>>
>>> * gnu/services/databases.scm (postgresql-shepherd-service): Replace
>>> make-forkexec-constructor and make-kill-destructor with pg_ctl.
>>
>> [...]
>>
>>> +     (let* ((pg_ctl-wrapper
>>> +             ;; Wrapper script that switches to the 'postgres' user before
>>> +             ;; launching daemon.
>>> +             (program-file
>>> +              "pg_ctl-wrapper"
>>> +              #~(begin
>>> +                  (use-modules (ice-9 match)
>>> +                               (ice-9 format))
>>> +                  (match (command-line)
>>> +                    ((_ mode)
>>> +                     (let ((user (getpwnam "postgres"))
>>> +                           (pg_ctl #$(file-append postgresql "/bin/pg_ctl")))
>>> +                       (setgid (passwd:gid user))
>>> +                       (setuid (passwd:uid user))
>>> +                       (system
>>> +                        (format #f "~a -D ~a -o '--config-file=~a -p ~d' ~a"
>>> +                                pg_ctl #$data-directory #$config-file #$port
>>> +                                mode))))))))
>>
>> I think we should use ‘execl’ here instead of ‘system’ so that (1) the
>> exit code is correct, and (2) we don’t go through /bin/sh.
>
> Hi Ludovic, thank you for the review.
>
> How do you pass single quotes as an ‘execl’ argument?
>
> https://www.postgresql.org/docs/9.3/static/app-pg-ctl.html says:
> --8<---------------cut here---------------start------------->8---
> -o options
>     Specifies options to be passed directly to the postgres command.
>     The options should usually be surrounded by single or double quotes
>     to ensure that they are passed through as a group.
> --8<---------------cut here---------------end--------------->8---

Oh I understood this.  I'll do a new patch.

> Also, I don't understand how the exit code matters since Guile scripts
> seem to always return 0, no matter if the last return value is true or
> false.
>
> Clément

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Clément Lassieur]

Fixes <https://bugs.gnu.org/29992>.

* gnu/services/databases.scm (postgresql-shepherd-service): Replace
make-forkexec-constructor and make-kill-destructor with pg_ctl.
---
 gnu/services/databases.scm | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index 6a01cb1ce..b34a67aa9 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Christopher Baines <mail@cbaines.net>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -147,26 +148,33 @@ host	all	all	::1/128 	trust"))
 (define postgresql-shepherd-service
   (match-lambda
     (($ <postgresql-configuration> postgresql port locale config-file data-directory)
-     (let ((start-script
-            ;; Wrapper script that switches to the 'postgres' user before
-            ;; launching daemon.
-            (program-file "start-postgres"
-                          #~(let ((user (getpwnam "postgres"))
-                                  (postgres (string-append #$postgresql
-                                                           "/bin/postgres")))
-                              (setgid (passwd:gid user))
-                              (setuid (passwd:uid user))
-                              (system* postgres
-                                       (string-append "--config-file="
-                                                      #$config-file)
-                                       "-p" (number->string #$port)
-                                       "-D" #$data-directory)))))
+     (let* ((pg_ctl-wrapper
+             ;; Wrapper script that switches to the 'postgres' user before
+             ;; launching daemon.
+             (program-file
+              "pg_ctl-wrapper"
+              #~(begin
+                  (use-modules (ice-9 match)
+                               (ice-9 format))
+                  (match (command-line)
+                    ((_ mode)
+                     (let ((user (getpwnam "postgres"))
+                           (pg_ctl #$(file-append postgresql "/bin/pg_ctl"))
+                           (options (format #f "--config-file=~a -p ~d"
+                                            #$config-file #$port)))
+                       (setgid (passwd:gid user))
+                       (setuid (passwd:uid user))
+                       (execl pg_ctl pg_ctl "-D" #$data-directory "-o" options
+                              mode)))))))
+            (action (lambda args
+                      #~(lambda _
+                          (invoke #$pg_ctl-wrapper #$@args)))))
        (list (shepherd-service
               (provision '(postgres))
               (documentation "Run the PostgreSQL daemon.")
               (requirement '(user-processes loopback syslogd))
-              (start #~(make-forkexec-constructor #$start-script))
-              (stop #~(make-kill-destructor))))))))
+              (start (action "start"))
+              (stop (action "stop"))))))))
 
 (define postgresql-service-type
   (service-type (name 'postgresql)
-- 
2.16.1

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Ludovic Courtès]

Hello,

Clément Lassieur <clement@lassieur.org> skribis:

> Fixes <https://bugs.gnu.org/29992>.
>
> * gnu/services/databases.scm (postgresql-shepherd-service): Replace
> make-forkexec-constructor and make-kill-destructor with pg_ctl.

LGTM, thanks!

In the future we should add a postgresql service test, perhaps it could
share code with the mysql test in (gnu tests databases).

Ludo’.

bug#29992: [PATCH] services: postgresql: Use pg_ctl to start and stop postgres.

[Clément Lassieur]

Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> Fixes <https://bugs.gnu.org/29992>.
>>
>> * gnu/services/databases.scm (postgresql-shepherd-service): Replace
>> make-forkexec-constructor and make-kill-destructor with pg_ctl.
>
> LGTM, thanks!

Pushed as 5ee4cd69c47b77e534654a130b1264ad05809943.

> In the future we should add a postgresql service test, perhaps it could
> share code with the mysql test in (gnu tests databases).

Uh-huh I was scared that you would ask this!  I'll add it to my todo
list then. :-)