Hello! On Fri, Nov 24, 2017 at 01:12:46AM +0200, Gena Makhomed wrote:
> On 23.11.2017 23:00, Maxim Dounin wrote: > > >>> Это всё замечательно (за вычетом того, предлагаемое использование > >>> daemon(3) почему-то не учитывает, что после вызова daemon(3) > >>> parent-процесса уже нет, а "ошибка" - не ошибка), но не отменяет > >>> того, что чуть менее, чем все существующие демоны делают именно > >>> "daemon(); write_pidfile();", и при таком подходе - ситуацию не > >>> изменить. > > >> А при каком подходе ситуацию c nginx изменить можно? > >> Если говорить конструктивно. > > > Чтобы изменить ситуацию конкретно с nginx - нужно сесть и сделать > > хороший патч. Очевидно, это сделать можно, и даже не очень > > сложно. Я, как уже неоднократно сказал, не возражаю. > > Для меня это будет слишком сложный патч, в разумные сроки не напишу. > > > Но сама идея, что все должны сесть и заняться выпиливанием > > стандартного паттерна, который работал десятки лет, и делать > > вместо это что-то своё с синхронизацией - не взлетит. > > Эта идея уже взлетела. Если демон состоит из одного процесса > - systemd может однозначно узнать его pid, проблемы могут возникать > только с теми демонами, которые состоят из нескольких процессов. > Из известных мне сервисов состоящих из более чем одного процесса: > > * postfix - сделали синхронизацию и проблем с systemd больше нет. > * httpd - перевели на Type=notify и проблем с systemd больше нет. > * php-fpm - перевели на Type=notify и проблем с systemd больше нет. > * nginx - только с этим сервисом наблюдаются проблемы под systemd. Давайте, всё-таки, опеределимся: мы за всё хорошее против всего плохого (== чтобы демоны писали pid-файлы до выхода запущенного процесса, потому что по другому - плохо), или вопрос исключительно в том, чтобы systemd не ругался в логи? Если за всё хорошее - то проблема, очевидно, не ограничевается сервисами из более чем одного процесса, и не решается переводом на Type=notify. Она вообще ортогональна существованию systemd. И идея её решения, очевидно, не взлетела, и уже наверное не взлетит. Если чтобы systemd не ругался - то проблема, очевидно, не в том, чтобы сделать хорошо, а в том, убрать конкретную ругань (которая не несёт практического смысла, см. ниже). И предолженный ранее workaround про sleep 0.1 - вполне себе в этом ключе же решение? > >> О каких именно "чуть менее, чем все существующие демоны" > >> сервисах Вы говорите? Есть еще кроме nginx примеры некорректного > >> поведения systemd-сервисов с Type=forking которые запускают много > >> дочерних процессов как это делает nginx или postfix? > > > Не вижу причин, почему демоны с "много дочерних процессов" должны > > отличаться от сервисов с "мало дочерних процессов". > > systemd однозначно определяет pid демонов состоящих из одного процесса > и поэтому для них в юнит-файле можно вообще не указывать опцию PIDFile= > - все будет работать как надо даже если они стартуют без синхронизации. > > Вот что говорит Lennart Poettering из Red Hat: > > If you use Type=forking, then you'll get away with not specifiying a > PID file in most cases, but it's racy as soon as you have more than > one daemon process, and nginx appears to be one of this kind, hence > please specify PIDFile=. > > https://lists.freedesktop.org/archives/systemd-devel/2017-November/039833.html Ну вот я посмотрел на это место чуть подробнее, и имею сказать, что это не совсем соответствует действительности. Единственное, для чего нужен PIDFile в случае nginx'а - это чтобы systemd нормально реагировал на binary upgrade. Для правильного детектирования основного процесса при запуске PIDFile не нужен, так как master-процесс - единственный, у кого parent'ом systemd, у остальных процессов parent'ом будет master. И соответственно все остальные процессы успешно отфильтрует вот этот код, https://github.com/systemd/systemd/blob/master/src/core/cgroup.c#L1727: /* Ignore processes that aren't our kids */ if (get_process_ppid(npid, &ppid) >= 0 && ppid != mypid) continue; Однако если PIDFile не указывать, то "service nginx upgrade" приведёт к тому, что после выхода старого мастера systemd будет считать, что nginx умер, и убьёт все новые процессы. Поэтому PIDFile указывать таки надо. Соответственно имеем то что имеем: PIDFile указывать надо, от этого на старте могут появляться сообщения про "PID file not ... yet?". Сообщения эти безвредные, и ни на что не влияют, кроме собственно появления самих сообщений. Если идти по пути синхронизации через pipe, то патч получается как-то такой. Не могу сказать, что он мне нравится, особенно в контексте решения задачи "чтобы у systemd в логе стало на одну строчку меньше". diff -r 325b3042edd6 src/core/nginx.c --- a/src/core/nginx.c Thu Nov 23 16:33:40 2017 +0300 +++ b/src/core/nginx.c Fri Nov 24 07:07:44 2017 +0300 @@ -361,6 +361,10 @@ return 1; } + if (ngx_daemon_sync(cycle->log) != NGX_OK) { + return 1; + } + if (ngx_log_redirect_stderr(cycle) != NGX_OK) { return 1; } diff -r 325b3042edd6 src/os/unix/ngx_daemon.c --- a/src/os/unix/ngx_daemon.c Thu Nov 23 16:33:40 2017 +0300 +++ b/src/os/unix/ngx_daemon.c Fri Nov 24 07:07:44 2017 +0300 @@ -9,10 +9,19 @@ #include <ngx_core.h> +static ngx_fd_t ngx_daemon_fd = NGX_INVALID_FILE; + + ngx_int_t ngx_daemon(ngx_log_t *log) { - int fd; + u_char buf[1]; + ngx_fd_t fd, pp[2]; + + if (pipe(pp) == -1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "pipe() failed"); + return NGX_ERROR; + } switch (fork()) { case -1: @@ -20,9 +29,30 @@ return NGX_ERROR; case 0: + if (close(pp[0]) == -1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "close() pipe failed"); + return NGX_ERROR; + } + + ngx_daemon_fd = pp[1]; break; default: + if (close(pp[1]) == -1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "close() pipe failed"); + return NGX_ERROR; + } + + if (read(pp[0], buf, 1) != 1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "read() pipe failed"); + return NGX_ERROR; + } + + if (close(pp[0]) == -1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "close() failed"); + return NGX_ERROR; + } + exit(0); } @@ -68,3 +98,26 @@ return NGX_OK; } + + +ngx_int_t +ngx_daemon_sync(ngx_log_t *log) +{ + if (ngx_daemon_fd == NGX_INVALID_FILE) { + return NGX_OK; + } + + if (write(ngx_daemon_fd, "", 1) != 1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "write() failed"); + return NGX_ERROR; + } + + if (close(ngx_daemon_fd) == -1) { + ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "close() failed"); + return NGX_ERROR; + } + + ngx_daemon_fd = NGX_INVALID_FILE; + + return NGX_OK; +} diff -r 325b3042edd6 src/os/unix/ngx_os.h --- a/src/os/unix/ngx_os.h Thu Nov 23 16:33:40 2017 +0300 +++ b/src/os/unix/ngx_os.h Fri Nov 24 07:07:44 2017 +0300 @@ -40,6 +40,7 @@ ngx_int_t ngx_os_specific_init(ngx_log_t *log); void ngx_os_specific_status(ngx_log_t *log); ngx_int_t ngx_daemon(ngx_log_t *log); +ngx_int_t ngx_daemon_sync(ngx_log_t *log); ngx_int_t ngx_os_signal_process(ngx_cycle_t *cycle, char *sig, ngx_pid_t pid); -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-ru mailing list nginx-ru@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-ru