Bug#728015: [nginx] Don't use type=forking in systemd service file

2013-11-09 Thread Adrien CLERC




If we turn this off, is it possible for systemd to send nginx the reload signal
as opposed to a complete restart?


Of course. If you look the attached service file, you can notice that 
the reload is fully operational. Here is a proof.


# Ensure that nginx is stopped first
root@complexe:~# systemctl stop nginx.service
root@complexe:~# pgrep nginx

# Start nginx and show actual PIDs
root@complexe:~# systemctl start nginx.service
root@complexe:~# systemctl status nginx.service
nginx.service - A high performance web server and a reverse proxy server
   Loaded: loaded (/etc/systemd/system/nginx.service; enabled)
[…]
 Main PID: 12876 (nginx)
   CGroup: name=systemd:/system/nginx.service
   ├─12876 nginx: master process /usr/sbin/nginx -g daemon off; 
master_process on;

   ├─12877 nginx: worker process
   ├─12878 nginx: worker process
   ├─12879 nginx: worker process
   └─12880 nginx: worker process

# Try with the reload command. Notice how worker PIDs change.
root@complexe:~# nginx -s reload
root@complexe:~# systemctl status nginx.service
nginx.service - A high performance web server and a reverse proxy server
[…]
 Main PID: 12876 (nginx)
   CGroup: name=systemd:/system/nginx.service
   ├─12876 nginx: master process /usr/sbin/nginx -g daemon off; 
master_process on;

   ├─12883 nginx: worker process
   ├─12884 nginx: worker process
   ├─12885 nginx: worker process
   └─12886 nginx: worker process

# Trying with systemctl. Notice how worker PIDs change also.
root@complexe:~# systemctl reload nginx.service
root@complexe:~# systemctl status nginx.service
nginx.service - A high performance web server and a reverse proxy server
[…]
 Main PID: 12876 (nginx)
   CGroup: name=systemd:/system/nginx.service
   ├─12876 nginx: master process /usr/sbin/nginx -g daemon off; 
master_process on;

   ├─12892 nginx: worker process
   ├─12893 nginx: worker process
   ├─12894 nginx: worker process
   └─12895 nginx: worker process

This is working because nginx writes its PID file anyway. Which is nice.



This would also break the nginx upgrade mechanism which is used for
package updates (postinst script) in order to upgrade the nginx binary
without dropping connections.

Using the current approach systemd understands that the pid has changed
correctly, probably by monitoring the pid file and the upgrade behaves
as expected. Actually there is a patch that will be included in the next
release that fixes this behaviour. [1]



Indeed, I've made some checks, that confirm a part of this. Type=forking 
can be removed, but PIDFile must be there, otherwise systemd thinks that 
since the main process is gone, the whole service should stop. With 
PIDFile, systemd reload the main PID from the file, and kill every other 
process if this main PID doesn't exist.


So to use nginx special ability to upgrade on-the-fly, at least PIDFile 
must be used. This would be great if the same PIDFile mentionned in the 
nginx's conf is used, but I don't actually see how this could be done.


And in this case, I don't have any special argument to remove the 
Type=forking directive, except that it's simpler. Not so convincing ;)



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#728015: [nginx] Don't use type=forking in systemd service file

2013-11-08 Thread Christos Trochalakis

On Sun, Oct 27, 2013 at 05:32:48PM +0100, Adrien CLERC wrote:

Package: nginx
Version: 1.4.3-2
Severity: minor

--- Please enter the report below this line. ---
Hi,

In the systemd service file, the two following directive are used:
Type=forking
PIDFile=/run/nginx.pid

However, when running systemd, it's easier to let systemd handle the 
PID stuff, and tell the program not to fork itself.
Moreover, we can have a situation when the user changes the PID file 
location in the configuration file located at /etc/nginx/nginx.conf


Thus, I recommend removing them, and disabling nginx forking with the 
directive daemon off. See attached unit file.


It is of course opened to discussion. I tried to find any side-effect 
of daemon off, but the documentation doesn't mention any.




This would also break the nginx upgrade mechanism which is used for
package updates (postinst script) in order to upgrade the nginx binary
without dropping connections.

Using the current approach systemd understands that the pid has changed
correctly, probably by monitoring the pid file and the upgrade behaves
as expected. Actually there is a patch that will be included in the next
release that fixes this behaviour. [1]

[0] http://nginx.org/en/docs/control.html Upgrading Executable on the
Fly Section
[1] 
http://anonscm.debian.org/gitweb/?p=collab-maint/nginx.git;a=commitdiff;h=1e277


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#728015: [nginx] Don't use type=forking in systemd service file

2013-10-27 Thread Adrien CLERC

Package: nginx
Version: 1.4.3-2
Severity: minor

--- Please enter the report below this line. ---
Hi,

In the systemd service file, the two following directive are used:
Type=forking
PIDFile=/run/nginx.pid

However, when running systemd, it's easier to let systemd handle the PID 
stuff, and tell the program not to fork itself.
Moreover, we can have a situation when the user changes the PID file 
location in the configuration file located at /etc/nginx/nginx.conf


Thus, I recommend removing them, and disabling nginx forking with the 
directive daemon off. See attached unit file.


It is of course opened to discussion. I tried to find any side-effect of 
daemon off, but the documentation doesn't mention any.


Have fun,

Adrien

--- System information. ---
Architecture: i386
Kernel: Linux 3.11-1-686-pae

Debian Release: jessie/sid
500 unstable ftp.fr.debian.org
1 experimental ftp.fr.debian.org

--- Package information. ---
Depends (Version) | Installed
=-+-=
nginx-common (= 1.4.3-2) | 1.4.3-2
libc6 (= 2.10) | 2.17-93
libpcre3 (= 8.10) | 1:8.31-2
libssl1.0.0 (= 1.0.1) | 1.0.1e-3
zlib1g (= 1:1.1.4) | 1:1.2.8.dfsg-1


Package's Recommends field is empty.

Suggests (Version) | Installed
=-+-
nginx-doc (= 1.4.3-2) |
[Unit]
Description=A high performance web server and a reverse proxy server
After=network.target

[Service]
ExecStartPre=/usr/sbin/nginx -t -q -g 'daemon off; master_process on;'
ExecStart=/usr/sbin/nginx -g 'daemon off; master_process on;'
ExecReload=/usr/sbin/nginx -g 'daemon off; master_process on;' -s reload
ExecStop=/usr/sbin/nginx -s quit

[Install]
WantedBy=multi-user.target


Bug#728015: [nginx] Don't use type=forking in systemd service file

2013-10-27 Thread Michael Lustfield
Using 'daemon off;' in nginx will likely cause issues when a user wants to run
'nginx -s reload'. The reload signal allows nginx to not drop any connections
while bringing up a new config. This is one of the reasons that using
supervisord is disliked so much.

This is done by opening a new master process that begins taking all of the new
connections while the old master is allowed to finish handling old connections.

If we turn this off, is it possible for systemd to send nginx the reload signal
as opposed to a complete restart?

On Sun, 27 Oct 2013 17:32:48 +0100
Adrien CLERC bugs-deb...@antipoul.fr wrote:

 Package: nginx
 Version: 1.4.3-2
 Severity: minor
 
 --- Please enter the report below this line. ---
 Hi,
 
 In the systemd service file, the two following directive are used:
 Type=forking
 PIDFile=/run/nginx.pid
 
 However, when running systemd, it's easier to let systemd handle the PID 
 stuff, and tell the program not to fork itself.
 Moreover, we can have a situation when the user changes the PID file 
 location in the configuration file located at /etc/nginx/nginx.conf
 
 Thus, I recommend removing them, and disabling nginx forking with the 
 directive daemon off. See attached unit file.
 
 It is of course opened to discussion. I tried to find any side-effect of 
 daemon off, but the documentation doesn't mention any.
 
 Have fun,
 
 Adrien
 
 --- System information. ---
 Architecture: i386
 Kernel: Linux 3.11-1-686-pae
 
 Debian Release: jessie/sid
 500 unstable ftp.fr.debian.org
 1 experimental ftp.fr.debian.org
 
 --- Package information. ---
 Depends (Version) | Installed
 =-+-=
 nginx-common (= 1.4.3-2) | 1.4.3-2
 libc6 (= 2.10) | 2.17-93
 libpcre3 (= 8.10) | 1:8.31-2
 libssl1.0.0 (= 1.0.1) | 1.0.1e-3
 zlib1g (= 1:1.1.4) | 1:1.2.8.dfsg-1
 
 
 Package's Recommends field is empty.
 
 Suggests (Version) | Installed
 =-+-
 nginx-doc (= 1.4.3-2) |


-- 
Michael Lustfield


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org