[PATCH] better systemd integration

Started by Peter Eisentrautabout 10 years ago19 messages
#1Peter Eisentraut
peter_e@gmx.net
3 attachment(s)

I have written a couple of patches to improve the integration of the
postgres daemon with systemd.

The setup that is shipped with Red Hat- and Debian-family packages at
the moment is just an imitation of the old shell scripts, relying on
polling by pg_ctl for readiness, with various custom pieces of
complexity for handling custom port numbers and such.

In the first patch, my proposal is to use sd_notify() calls from
libsystemd to notify the systemd daemon directly when startup is
completed. This is the recommended low-overhead solution that is now
being adopted by many other server packages. It allows us to cut out
pg_ctl completely from the startup configuration and makes the startup
configuration manageable by non-wizards. An example is included in the
patch.

The second patch improves integration with the system journal managed by
systemd. This is a facility that captures a daemon's standard output
and error and records it in configurable places, including syslog. The
patch adds a new log_destination that is like stderr but marks up the
output so that systemd knows the severity. With that in place, users
can choose to do away with the postgres log file management and let
systemd do it.

The third patch is technically unrelated but arose while I was working
on this. It improves error reporting when the data directory is missing.

Attachments:

0001-Add-support-for-systemd-service-notifications.patchapplication/x-patch; name=0001-Add-support-for-systemd-service-notifications.patchDownload
From c1725cbf51b79cfd78bc7f9358891599c4ffae6c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:46:17 -0500
Subject: [PATCH 1/3] Add support for systemd service notifications

Insert sd_notify() calls at server start and stop for integration with
systemd.  This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
---
 configure                           | 38 +++++++++++++++++++++++++++++++++++++
 configure.in                        |  9 +++++++++
 doc/src/sgml/installation.sgml      | 16 ++++++++++++++++
 doc/src/sgml/runtime.sgml           | 24 +++++++++++++++++++++++
 src/Makefile.global.in              |  1 +
 src/backend/Makefile                |  4 ++++
 src/backend/postmaster/postmaster.c | 21 ++++++++++++++++++++
 src/include/pg_config.h.in          |  3 +++
 8 files changed, 116 insertions(+)

diff --git a/configure b/configure
index b771a83..f9f93b3 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ with_libxml
 XML2_CONFIG
 UUID_EXTRA_OBJS
 with_uuid
+with_systemd
 with_selinux
 with_openssl
 krb_srvtab
@@ -830,6 +831,7 @@ with_ldap
 with_bonjour
 with_openssl
 with_selinux
+with_systemd
 with_readline
 with_libedit_preferred
 with_uuid
@@ -1518,6 +1520,7 @@ Optional Packages:
   --with-bonjour          build with Bonjour support
   --with-openssl          build with OpenSSL support
   --with-selinux          build with SELinux support
+  --with-systemd          build with systemd support
   --without-readline      do not use GNU Readline nor BSD Libedit for editing
   --with-libedit-preferred
                           prefer BSD Libedit over GNU Readline
@@ -5695,6 +5698,41 @@ fi
 $as_echo "$with_selinux" >&6; }
 
 #
+# Systemd
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with systemd support" >&5
+$as_echo_n "checking whether to build with systemd support... " >&6; }
+
+
+
+# Check whether --with-systemd was given.
+if test "${with_systemd+set}" = set; then :
+  withval=$with_systemd;
+  case $withval in
+    yes)
+
+$as_echo "#define USE_SYSTEMD 1" >>confdefs.h
+
+      ;;
+    no)
+      :
+      ;;
+    *)
+      as_fn_error $? "no argument expected for --with-systemd option" "$LINENO" 5
+      ;;
+  esac
+
+else
+  with_systemd=no
+
+fi
+
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_systemd" >&5
+$as_echo "$with_systemd" >&6; }
+
+#
 # Readline
 #
 
diff --git a/configure.in b/configure.in
index b5868b0..47c82cb 100644
--- a/configure.in
+++ b/configure.in
@@ -700,6 +700,15 @@ AC_SUBST(with_selinux)
 AC_MSG_RESULT([$with_selinux])
 
 #
+# Systemd
+#
+AC_MSG_CHECKING([whether to build with systemd support])
+PGAC_ARG_BOOL(with, systemd, no, [build with systemd support],
+              [AC_DEFINE([USE_SYSTEMD], 1, [Define to build with systemd support. (--with-systemd)])])
+AC_SUBST(with_systemd)
+AC_MSG_RESULT([$with_systemd])
+
+#
 # Readline
 #
 PGAC_ARG_BOOL(with, readline, yes,
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 8964834..7b6a389 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -813,6 +813,22 @@ <title>Configuration</title>
       </varlistentry>
 
       <varlistentry>
+       <term><option>--with-systemd</option></term>
+       <listitem>
+        <para>
+         Build with support
+         for <application>systemd</application><indexterm><primary>systemd</primary></indexterm>
+         service notifications.  This improves integration if the server binary
+         is started under <application>systemd</application> but has no impact
+         otherwise; see <xref linkend="server-start"> for more
+         information.  <application>libsystemd</application> and the
+         associated header files need to be installed to be able to use this
+         option.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term><option>--without-readline</option></term>
        <listitem>
         <para>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 6d5b108..5e95a3b 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -369,6 +369,30 @@ <title>Starting the Database Server</title>
       <filename>contrib/start-scripts/linux</filename> in the
       <productname>PostgreSQL</productname> source distribution.
      </para>
+
+     <para>
+      When using <application>systemd</application>, you can use the following
+      service unit file (e.g.,
+      at <filename>/etc/systemd/system/postgresql.service</filename>):<indexterm><primary>systemd</primary></indexterm>
+<programlisting>
+[Unit]
+Description=PostgreSQL database server
+Documentation=man:postgres(1)
+
+[Service]
+Type=notify
+User=postgres
+ExecStart=/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data
+ExecReload=/bin/kill -HUP $MAINPID
+KillMode=mixed
+KillSignal=SIGINT
+
+[Install]
+WantedBy=multi-user.target
+</programlisting>
+      Using <literal>Type=notify</literal> requires that the server binary was
+      built with <literal>configure --with-systemd</literal>.
+     </para>
     </listitem>
 
     <listitem>
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 51f4797..e94d6a5 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -184,6 +184,7 @@ with_python	= @with_python@
 with_tcl	= @with_tcl@
 with_openssl	= @with_openssl@
 with_selinux	= @with_selinux@
+with_systemd	= @with_systemd@
 with_libxml	= @with_libxml@
 with_libxslt	= @with_libxslt@
 with_system_tzdata = @with_system_tzdata@
diff --git a/src/backend/Makefile b/src/backend/Makefile
index fb60420..455e50b 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -45,6 +45,10 @@ LIBS := $(filter-out -lpgport -lpgcommon, $(LIBS)) $(LDAP_LIBS_BE)
 # The backend doesn't need everything that's in LIBS, however
 LIBS := $(filter-out -lz -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
 
+ifeq ($(with_systemd),yes)
+LIBS += -lsystemd
+endif
+
 ##########################################################################
 
 all: submake-libpgport submake-schemapg postgres $(POSTGRES_IMP)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3cba0e5..b23e483 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -87,6 +87,10 @@
 #include <dns_sd.h>
 #endif
 
+#ifdef USE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
+
 #ifdef HAVE_PTHREAD_IS_THREADED_NP
 #include <pthread.h>
 #endif
@@ -2533,6 +2537,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = SmartShutdown;
 			ereport(LOG,
 					(errmsg("received smart shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			if (pmState == PM_RUN || pmState == PM_RECOVERY ||
 				pmState == PM_HOT_STANDBY || pmState == PM_STARTUP)
@@ -2585,6 +2592,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = FastShutdown;
 			ereport(LOG,
 					(errmsg("received fast shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			if (StartupPID != 0)
 				signal_child(StartupPID, SIGTERM);
@@ -2645,6 +2655,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = ImmediateShutdown;
 			ereport(LOG,
 					(errmsg("received immediate shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			TerminateChildren(SIGQUIT);
 			pmState = PM_WAIT_BACKENDS;
@@ -2787,6 +2800,10 @@ reaper(SIGNAL_ARGS)
 			ereport(LOG,
 				 (errmsg("database system is ready to accept connections")));
 
+#ifdef USE_SYSTEMD
+			sd_notify(0, "READY=1");
+#endif
+
 			continue;
 		}
 
@@ -4928,6 +4945,10 @@ sigusr1_handler(SIGNAL_ARGS)
 		ereport(LOG,
 		(errmsg("database system is ready to accept read only connections")));
 
+#ifdef USE_SYSTEMD
+		sd_notify(0, "READY=1");
+#endif
+
 		pmState = PM_HOT_STANDBY;
 		/* Some workers may be scheduled to start now */
 		StartWorkerNeeded = true;
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index a20e0cd..110ecb1 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -830,6 +830,9 @@
 /* Define to 1 to use Intel SSSE 4.2 CRC instructions with a runtime check. */
 #undef USE_SSE42_CRC32C_WITH_RUNTIME_CHECK
 
+/* Define to build with systemd support. (--with-systemd) */
+#undef USE_SYSTEMD
+
 /* Define to select SysV-style semaphores. */
 #undef USE_SYSV_SEMAPHORES
 
-- 
2.6.3

0002-Add-log_destination-systemd.patchapplication/x-patch; name=0002-Add-log_destination-systemd.patchDownload
From ea46337f24df4d251243d4ea923c5ff1e2f65e96 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:46:44 -0500
Subject: [PATCH 2/3] Add log_destination "systemd"

This allows systemd to associate severity levels with messages printed
to standard output or standard error.
---
 doc/src/sgml/config.sgml                      | 14 +++++++++-
 doc/src/sgml/runtime.sgml                     |  2 +-
 src/backend/utils/error/elog.c                | 37 +++++++++++++++++++++++++++
 src/backend/utils/misc/guc.c                  |  4 ++-
 src/backend/utils/misc/postgresql.conf.sample |  7 ++---
 src/include/utils/elog.h                      |  1 +
 6 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6e14851..a61d179 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3821,7 +3821,7 @@ <title>Where To Log</title>
        <para>
         <productname>PostgreSQL</productname> supports several methods
          for logging server messages, including
-         <systemitem>stderr</systemitem>, <systemitem>csvlog</systemitem> and
+         <systemitem>stderr</systemitem>, <systemitem>systemd</systemitem>, <systemitem>csvlog</systemitem> and
          <systemitem>syslog</systemitem>. On Windows,
          <systemitem>eventlog</systemitem> is also supported. Set this
          parameter to a list of desired log destinations separated by
@@ -3831,6 +3831,18 @@ <title>Where To Log</title>
          file or on the server command line.
        </para>
        <para>
+        <systemitem>systemd</systemitem><indexterm><primary>systemd</primary></indexterm>
+        is like <systemitem>stderr</systemitem>, but log messages are
+        additionally prefixed so that <application>systemd</application> can
+        associate severity levels
+        (see <citerefentry><refentrytitle>sd-daemon</refentrytitle><manvolnum>3</manvolnum></citerefentry>).
+        This is recommended if the server is started and logs are captured
+        by <application>systemd</application>.  It is normally not useful to
+        configure <systemitem>systemd</systemitem> together
+        with <systemitem>stderr</systemitem>
+        or <systemitem>syslog</systemitem>.
+       </para>
+       <para>
         If <systemitem>csvlog</> is included in <varname>log_destination</>,
         log entries are output in <quote>comma separated
         value</> (<acronym>CSV</>) format, which is convenient for
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 5e95a3b..0773610 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -382,7 +382,7 @@ <title>Starting the Database Server</title>
 [Service]
 Type=notify
 User=postgres
-ExecStart=/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data
+ExecStart=/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data -c log_destination=systemd
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=mixed
 KillSignal=SIGINT
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b5b6c5..12e6cea 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -3004,6 +3004,43 @@ send_message_to_server_log(ErrorData *edata)
 			write_console(buf.data, buf.len);
 	}
 
+	if (Log_destination & LOG_DESTINATION_SYSTEMD)
+	{
+		const char *systemd_log_prefix;
+
+		switch (edata->elevel)
+		{
+			case DEBUG5:
+			case DEBUG4:
+			case DEBUG3:
+			case DEBUG2:
+			case DEBUG1:
+				systemd_log_prefix = "<7>" /* SD_DEBUG */;
+				break;
+			case LOG:
+			case COMMERROR:
+			case INFO:
+				systemd_log_prefix = "<6>" /* SD_INFO */;
+				break;
+			case NOTICE:
+			case WARNING:
+				systemd_log_prefix = "<5>" /* SD_NOTICE */;
+				break;
+			case ERROR:
+				systemd_log_prefix = "<4>" /* SD_WARNING */;
+				break;
+			case FATAL:
+				systemd_log_prefix = "<3>" /* SD_ERR */;
+				break;
+			case PANIC:
+			default:
+				systemd_log_prefix = "<2>" /* LOG_CRIT */;
+				break;
+		}
+		write_console(systemd_log_prefix, strlen(systemd_log_prefix));
+		write_console(buf.data, buf.len);
+	}
+
 	/* If in the syslogger process, try to write messages direct to file */
 	if (am_syslogger)
 		write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_STDERR);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a185749..c9a06bd 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3123,7 +3123,7 @@ static struct config_string ConfigureNamesString[] =
 		{"log_destination", PGC_SIGHUP, LOGGING_WHERE,
 			gettext_noop("Sets the destination for server log output."),
 			gettext_noop("Valid values are combinations of \"stderr\", "
-						 "\"syslog\", \"csvlog\", and \"eventlog\", "
+						 "\"syslog\", \"systemd\", \"csvlog\", and \"eventlog\", "
 						 "depending on the platform."),
 			GUC_LIST_INPUT
 		},
@@ -9694,6 +9694,8 @@ check_log_destination(char **newval, void **extra, GucSource source)
 
 		if (pg_strcasecmp(tok, "stderr") == 0)
 			newlogdest |= LOG_DESTINATION_STDERR;
+		else if (pg_strcasecmp(tok, "systemd") == 0)
+			newlogdest |= LOG_DESTINATION_SYSTEMD;
 		else if (pg_strcasecmp(tok, "csvlog") == 0)
 			newlogdest |= LOG_DESTINATION_CSVLOG;
 #ifdef HAVE_SYSLOG
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 029114f..2af7465 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -322,9 +322,10 @@
 # - Where to Log -
 
 #log_destination = 'stderr'		# Valid values are combinations of
-					# stderr, csvlog, syslog, and eventlog,
-					# depending on platform.  csvlog
-					# requires logging_collector to be on.
+					# stderr, systemd, csvlog, syslog, and
+					# eventlog, depending on platform.
+					# csvlog requires logging_collector to
+					# be on.
 
 # This is used when logging to stderr:
 #logging_collector = off		# Enable capturing of stderr and csvlog
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7715719..b816e82 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -402,6 +402,7 @@ extern char *Log_destination_string;
 #define LOG_DESTINATION_SYSLOG	 2
 #define LOG_DESTINATION_EVENTLOG 4
 #define LOG_DESTINATION_CSVLOG	 8
+#define LOG_DESTINATION_SYSTEMD	 16
 
 /* Other exported functions */
 extern void DebugFileOpen(void);
-- 
2.6.3

0003-Improve-error-reporting-when-location-specified-by-p.patchapplication/x-patch; name=0003-Improve-error-reporting-when-location-specified-by-p.patchDownload
From 661989f7912a8d1df84bcb798fb4348a8114c291 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:47:18 -0500
Subject: [PATCH 3/3] Improve error reporting when location specified by
 postgres -D does not exist

Previously, the first error seen would be that postgresql.conf does not
exist.  But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
---
 src/backend/utils/misc/guc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c9a06bd..d9de2da 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4463,6 +4463,17 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	else
 		configdir = make_absolute_path(getenv("PGDATA"));
 
+	if (configdir && stat(configdir, &stat_buf) != 0)
+	{
+		write_stderr("%s: could not access \"%s\": %s\n",
+					 progname,
+					 configdir,
+					 strerror(errno));
+		if (errno == ENOENT)
+			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		return false;
+	}
+
 	/*
 	 * Find the configuration file: if config_file was specified on the
 	 * command line, use it, else use configdir/postgresql.conf.  In any case
@@ -4498,7 +4509,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 */
 	if (stat(ConfigFileName, &stat_buf) != 0)
 	{
-		write_stderr("%s cannot access the server configuration file \"%s\": %s\n",
+		write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
 					 progname, ConfigFileName, strerror(errno));
 		free(configdir);
 		return false;
-- 
2.6.3

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: [PATCH] better systemd integration

Peter Eisentraut <peter_e@gmx.net> writes:

I have written a couple of patches to improve the integration of the
postgres daemon with systemd.

Seems like a generally reasonable thing to do. systemd is probably
not going away (unfortunately IMO, but there it is).

The second patch improves integration with the system journal managed by
systemd. This is a facility that captures a daemon's standard output
and error and records it in configurable places, including syslog. The
patch adds a new log_destination that is like stderr but marks up the
output so that systemd knows the severity. With that in place, users
can choose to do away with the postgres log file management and let
systemd do it.

One of the benefits of the log collector is that it's able to do something
reasonably sane with random stderr output that might be generated by
libraries linked into PG (starting with glibc...). If someone sets things
up as you're suggesting, what will systemd do with unlabeled output lines?
Or in other words, how much value-add is there really from this markup?

Also, it looks like the markup is intended to be per-line, but as you've
coded this a possibly-multi-line error report will only have a prefix
on the first line.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: [PATCH] better systemd integration

Peter Eisentraut wrote:

I have written a couple of patches to improve the integration of the
postgres daemon with systemd.

Great. Is anything happening with these patches, or? They've been
inactive for quite a while now.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Alvaro Herrera (#3)
Re: [PATCH] better systemd integration

On 1/18/16 10:58 AM, Alvaro Herrera wrote:

Peter Eisentraut wrote:

I have written a couple of patches to improve the integration of the
postgres daemon with systemd.

Great. Is anything happening with these patches, or? They've been
inactive for quite a while now.

Well, they are waiting for someone to review them.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#4)
Re: [PATCH] better systemd integration

2016-01-21 3:33 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:

On 1/18/16 10:58 AM, Alvaro Herrera wrote:

Peter Eisentraut wrote:

I have written a couple of patches to improve the integration of the
postgres daemon with systemd.

Great. Is anything happening with these patches, or? They've been
inactive for quite a while now.

Well, they are waiting for someone to review them.

I read some basic materials about systemd and these patche looks correct.
Next week I'll test it.

Regards

Pavel

Show quoted text

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#1)
Re: [PATCH] better systemd integration

Hi

2015-11-17 15:08 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:

I have written a couple of patches to improve the integration of the
postgres daemon with systemd.

The setup that is shipped with Red Hat- and Debian-family packages at
the moment is just an imitation of the old shell scripts, relying on
polling by pg_ctl for readiness, with various custom pieces of
complexity for handling custom port numbers and such.

In the first patch, my proposal is to use sd_notify() calls from
libsystemd to notify the systemd daemon directly when startup is
completed. This is the recommended low-overhead solution that is now
being adopted by many other server packages. It allows us to cut out
pg_ctl completely from the startup configuration and makes the startup
configuration manageable by non-wizards. An example is included in the
patch.

The second patch improves integration with the system journal managed by
systemd. This is a facility that captures a daemon's standard output
and error and records it in configurable places, including syslog. The
patch adds a new log_destination that is like stderr but marks up the
output so that systemd knows the severity. With that in place, users
can choose to do away with the postgres log file management and let
systemd do it.

The third patch is technically unrelated but arose while I was working
on this. It improves error reporting when the data directory is missing.

2. all tests passed

The issues:

1. configure missing systemd integration test, compilation fails:

postmaster.o postmaster.c
postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
directory

3. PostgreSQL is able to write to systemd log, but multiline entry was
stored with different priorities

do $$ begin raise warning 'NAZDAREK****'; end $$;

first line

{
"__CURSOR" :
"s=cac797bc03f242febea9f32357bba773;i=b4a5;b=e8d5b3df2ebf46dd86c39046b326bd32;m=1cb792a63b;t=52a4f3ad40860;x=57014959bf6e3481",
"__REALTIME_TIMESTAMP" : "1453894661310560",
"__MONOTONIC_TIMESTAMP" : "123338925627",
"_BOOT_ID" : "e8d5b3df2ebf46dd86c39046b326bd32",
"SYSLOG_FACILITY" : "3",
"_UID" : "1001",
"_GID" : "1001",
"_CAP_EFFECTIVE" : "0",
"_SELINUX_CONTEXT" : "system_u:system_r:init_t:s0",
"_MACHINE_ID" : "b8299a722638414a8776d3e130e228e4",
"_HOSTNAME" : "localhost.localdomain",
"_SYSTEMD_SLICE" : "system.slice",
"_TRANSPORT" : "stdout",
"SYSLOG_IDENTIFIER" : "postgres",
"_PID" : "3150",
"_COMM" : "postgres",
"_EXE" : "/usr/local/pgsql/bin/postgres",
"_CMDLINE" : "/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data -c
log_destination=systemd",
"_SYSTEMD_CGROUP" : "/system.slice/postgresql.service",
"_SYSTEMD_UNIT" : "postgresql.service",
"PRIORITY" : "5",
"MESSAGE" : "WARNING: NAZDAREK****"
}

second line

{
"__CURSOR" :
"s=cac797bc03f242febea9f32357bba773;i=b4a6;b=e8d5b3df2ebf46dd86c39046b326bd32;m=1cb792a882;t=52a4f3ad40aa6;x=ae9801b2ecbd4da3",
"__REALTIME_TIMESTAMP" : "1453894661311142",
"__MONOTONIC_TIMESTAMP" : "123338926210",
"_BOOT_ID" : "e8d5b3df2ebf46dd86c39046b326bd32",
"PRIORITY" : "6",
"SYSLOG_FACILITY" : "3",
"_UID" : "1001",
"_GID" : "1001",
"_CAP_EFFECTIVE" : "0",
"_SELINUX_CONTEXT" : "system_u:system_r:init_t:s0",
"_MACHINE_ID" : "b8299a722638414a8776d3e130e228e4",
"_HOSTNAME" : "localhost.localdomain",
"_SYSTEMD_SLICE" : "system.slice",
"_TRANSPORT" : "stdout",
"SYSLOG_IDENTIFIER" : "postgres",
"_PID" : "3150",
"_COMM" : "postgres",
"_EXE" : "/usr/local/pgsql/bin/postgres",
"_CMDLINE" : "/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data -c
log_destination=systemd",
"_SYSTEMD_CGROUP" : "/system.slice/postgresql.service",
"_SYSTEMD_UNIT" : "postgresql.service",
"MESSAGE" : "CONTEXT: PL/pgSQL function inline_code_block line 1 at
RAISE"
}

Is it expected?

Second issue:

Mapping of levels between pg and journal levels is moved by1

+            case DEBUG1:
+                systemd_log_prefix = "<7>" /* SD_DEBUG */;
+                break;
+            case LOG:
+            case COMMERROR:
+            case INFO:
+                systemd_log_prefix = "<6>" /* SD_INFO */;
+                break;
+            case NOTICE:
+            case WARNING:
+                systemd_log_prefix = "<5>" /* SD_NOTICE */;
+                break;
+            case ERROR:
+                systemd_log_prefix = "<4>" /* SD_WARNING */;
+                break;
+            case FATAL:
+                systemd_log_prefix = "<3>" /* SD_ERR */;
+                break;
+            case PANIC:

is it expected?

This is little bit unexpected - (can be correct).

When I use filtering "warnings", then I got errors, etc. I can understand
so these systems are not compatible, but these differences should be well
documented.

I didn't find any other issues. It is working without any problems.

Regards

Pavel

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#6)
2 attachment(s)
Re: [PATCH] better systemd integration

On 1/27/16 7:02 AM, Pavel Stehule wrote:

The issues:

1. configure missing systemd integration test, compilation fails:

postmaster.o postmaster.c
postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
directory

Updated patch attached that fixes this by adding additional checking in
configure.

3. PostgreSQL is able to write to systemd log, but multiline entry was
stored with different priorities

Yeah, as Tom had already pointed out, this doesn't work as I had
imagined it. I'm withdrawing this part of the patch for now. I'll come
back to it later.

Second issue:

Mapping of levels between pg and journal levels is moved by1

This is the same as how the "syslog" destination works.

Attachments:

0001-Improve-error-reporting-when-location-specified-by-p.patchapplication/x-patch; name=0001-Improve-error-reporting-when-location-specified-by-p.patchDownload
From 607323c95ca62d74668992219569c7cff470b63d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:47:18 -0500
Subject: [PATCH 1/3] Improve error reporting when location specified by
 postgres -D does not exist

Previously, the first error seen would be that postgresql.conf does not
exist.  But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
---
 src/backend/utils/misc/guc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 38ba82f..b8d34b5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4463,6 +4463,17 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	else
 		configdir = make_absolute_path(getenv("PGDATA"));
 
+	if (configdir && stat(configdir, &stat_buf) != 0)
+	{
+		write_stderr("%s: could not access \"%s\": %s\n",
+					 progname,
+					 configdir,
+					 strerror(errno));
+		if (errno == ENOENT)
+			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		return false;
+	}
+
 	/*
 	 * Find the configuration file: if config_file was specified on the
 	 * command line, use it, else use configdir/postgresql.conf.  In any case
@@ -4498,7 +4509,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 */
 	if (stat(ConfigFileName, &stat_buf) != 0)
 	{
-		write_stderr("%s cannot access the server configuration file \"%s\": %s\n",
+		write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
 					 progname, ConfigFileName, strerror(errno));
 		free(configdir);
 		return false;
-- 
2.7.0

0002-Add-support-for-systemd-service-notifications.patchapplication/x-patch; name=0002-Add-support-for-systemd-service-notifications.patchDownload
From 339836d39d8566ed794f6e1d56384fd93073d5bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:46:17 -0500
Subject: [PATCH 2/3] Add support for systemd service notifications

Insert sd_notify() calls at server start and stop for integration with
systemd.  This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
---
 configure                           | 38 +++++++++++++++++++++++++++++++++++++
 configure.in                        |  9 +++++++++
 doc/src/sgml/installation.sgml      | 16 ++++++++++++++++
 doc/src/sgml/runtime.sgml           | 24 +++++++++++++++++++++++
 src/Makefile.global.in              |  1 +
 src/backend/Makefile                |  4 ++++
 src/backend/postmaster/postmaster.c | 21 ++++++++++++++++++++
 src/include/pg_config.h.in          |  3 +++
 8 files changed, 116 insertions(+)

diff --git a/configure b/configure
index 3dd1b15..5c2e767 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ with_libxml
 XML2_CONFIG
 UUID_EXTRA_OBJS
 with_uuid
+with_systemd
 with_selinux
 with_openssl
 krb_srvtab
@@ -830,6 +831,7 @@ with_ldap
 with_bonjour
 with_openssl
 with_selinux
+with_systemd
 with_readline
 with_libedit_preferred
 with_uuid
@@ -1518,6 +1520,7 @@ Optional Packages:
   --with-bonjour          build with Bonjour support
   --with-openssl          build with OpenSSL support
   --with-selinux          build with SELinux support
+  --with-systemd          build with systemd support
   --without-readline      do not use GNU Readline nor BSD Libedit for editing
   --with-libedit-preferred
                           prefer BSD Libedit over GNU Readline
@@ -5695,6 +5698,41 @@ fi
 $as_echo "$with_selinux" >&6; }
 
 #
+# Systemd
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with systemd support" >&5
+$as_echo_n "checking whether to build with systemd support... " >&6; }
+
+
+
+# Check whether --with-systemd was given.
+if test "${with_systemd+set}" = set; then :
+  withval=$with_systemd;
+  case $withval in
+    yes)
+
+$as_echo "#define USE_SYSTEMD 1" >>confdefs.h
+
+      ;;
+    no)
+      :
+      ;;
+    *)
+      as_fn_error $? "no argument expected for --with-systemd option" "$LINENO" 5
+      ;;
+  esac
+
+else
+  with_systemd=no
+
+fi
+
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_systemd" >&5
+$as_echo "$with_systemd" >&6; }
+
+#
 # Readline
 #
 
diff --git a/configure.in b/configure.in
index 9398482..20d9fe1 100644
--- a/configure.in
+++ b/configure.in
@@ -700,6 +700,15 @@ AC_SUBST(with_selinux)
 AC_MSG_RESULT([$with_selinux])
 
 #
+# Systemd
+#
+AC_MSG_CHECKING([whether to build with systemd support])
+PGAC_ARG_BOOL(with, systemd, no, [build with systemd support],
+              [AC_DEFINE([USE_SYSTEMD], 1, [Define to build with systemd support. (--with-systemd)])])
+AC_SUBST(with_systemd)
+AC_MSG_RESULT([$with_systemd])
+
+#
 # Readline
 #
 PGAC_ARG_BOOL(with, readline, yes,
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 8964834..7b6a389 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -813,6 +813,22 @@ <title>Configuration</title>
       </varlistentry>
 
       <varlistentry>
+       <term><option>--with-systemd</option></term>
+       <listitem>
+        <para>
+         Build with support
+         for <application>systemd</application><indexterm><primary>systemd</primary></indexterm>
+         service notifications.  This improves integration if the server binary
+         is started under <application>systemd</application> but has no impact
+         otherwise; see <xref linkend="server-start"> for more
+         information.  <application>libsystemd</application> and the
+         associated header files need to be installed to be able to use this
+         option.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term><option>--without-readline</option></term>
        <listitem>
         <para>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index cda05f5..58b8838 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -369,6 +369,30 @@ <title>Starting the Database Server</title>
       <filename>contrib/start-scripts/linux</filename> in the
       <productname>PostgreSQL</productname> source distribution.
      </para>
+
+     <para>
+      When using <application>systemd</application>, you can use the following
+      service unit file (e.g.,
+      at <filename>/etc/systemd/system/postgresql.service</filename>):<indexterm><primary>systemd</primary></indexterm>
+<programlisting>
+[Unit]
+Description=PostgreSQL database server
+Documentation=man:postgres(1)
+
+[Service]
+Type=notify
+User=postgres
+ExecStart=/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data
+ExecReload=/bin/kill -HUP $MAINPID
+KillMode=mixed
+KillSignal=SIGINT
+
+[Install]
+WantedBy=multi-user.target
+</programlisting>
+      Using <literal>Type=notify</literal> requires that the server binary was
+      built with <literal>configure --with-systemd</literal>.
+     </para>
     </listitem>
 
     <listitem>
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 51f4797..e94d6a5 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -184,6 +184,7 @@ with_python	= @with_python@
 with_tcl	= @with_tcl@
 with_openssl	= @with_openssl@
 with_selinux	= @with_selinux@
+with_systemd	= @with_systemd@
 with_libxml	= @with_libxml@
 with_libxslt	= @with_libxslt@
 with_system_tzdata = @with_system_tzdata@
diff --git a/src/backend/Makefile b/src/backend/Makefile
index d4db8ff..b3d5e2e 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -45,6 +45,10 @@ LIBS := $(filter-out -lpgport -lpgcommon, $(LIBS)) $(LDAP_LIBS_BE)
 # The backend doesn't need everything that's in LIBS, however
 LIBS := $(filter-out -lz -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
 
+ifeq ($(with_systemd),yes)
+LIBS += -lsystemd
+endif
+
 ##########################################################################
 
 all: submake-libpgport submake-schemapg postgres $(POSTGRES_IMP)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9aaed5b..2e7f1d7 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -87,6 +87,10 @@
 #include <dns_sd.h>
 #endif
 
+#ifdef USE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
+
 #ifdef HAVE_PTHREAD_IS_THREADED_NP
 #include <pthread.h>
 #endif
@@ -2533,6 +2537,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = SmartShutdown;
 			ereport(LOG,
 					(errmsg("received smart shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			if (pmState == PM_RUN || pmState == PM_RECOVERY ||
 				pmState == PM_HOT_STANDBY || pmState == PM_STARTUP)
@@ -2585,6 +2592,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = FastShutdown;
 			ereport(LOG,
 					(errmsg("received fast shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			if (StartupPID != 0)
 				signal_child(StartupPID, SIGTERM);
@@ -2645,6 +2655,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = ImmediateShutdown;
 			ereport(LOG,
 					(errmsg("received immediate shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			TerminateChildren(SIGQUIT);
 			pmState = PM_WAIT_BACKENDS;
@@ -2787,6 +2800,10 @@ reaper(SIGNAL_ARGS)
 			ereport(LOG,
 				 (errmsg("database system is ready to accept connections")));
 
+#ifdef USE_SYSTEMD
+			sd_notify(0, "READY=1");
+#endif
+
 			continue;
 		}
 
@@ -4930,6 +4947,10 @@ sigusr1_handler(SIGNAL_ARGS)
 		ereport(LOG,
 		(errmsg("database system is ready to accept read only connections")));
 
+#ifdef USE_SYSTEMD
+		sd_notify(0, "READY=1");
+#endif
+
 		pmState = PM_HOT_STANDBY;
 		/* Some workers may be scheduled to start now */
 		StartWorkerNeeded = true;
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 16a272e..b3ceea5 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -833,6 +833,9 @@
 /* Define to 1 to use Intel SSSE 4.2 CRC instructions with a runtime check. */
 #undef USE_SSE42_CRC32C_WITH_RUNTIME_CHECK
 
+/* Define to build with systemd support. (--with-systemd) */
+#undef USE_SYSTEMD
+
 /* Define to select SysV-style semaphores. */
 #undef USE_SYSV_SEMAPHORES
 
-- 
2.7.0

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#7)
Re: [PATCH] better systemd integration

Hi

2016-01-28 3:50 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:

On 1/27/16 7:02 AM, Pavel Stehule wrote:

The issues:

1. configure missing systemd integration test, compilation fails:

postmaster.o postmaster.c
postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
directory

Updated patch attached that fixes this by adding additional checking in
configure.

You sent only rebased code of previous version. I didn't find additional
checks.

3. PostgreSQL is able to write to systemd log, but multiline entry was
stored with different priorities

Yeah, as Tom had already pointed out, this doesn't work as I had
imagined it. I'm withdrawing this part of the patch for now. I'll come
back to it later.

ok

Second issue:

Mapping of levels between pg and journal levels is moved by1

This is the same as how the "syslog" destination works.

I didn't find any related code in PostgreSQL, can me help, please?

Regards

Pavel

#9Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#7)
Re: [PATCH] better systemd integration

Second issue:

Mapping of levels between pg and journal levels is moved by1

This is the same as how the "syslog" destination works.

I understand to this logic, but I miss any documentation.

Regards

Pavel

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#8)
2 attachment(s)
Re: [PATCH] better systemd integration

On 1/28/16 4:00 AM, Pavel Stehule wrote:

Hi

2016-01-28 3:50 GMT+01:00 Peter Eisentraut <peter_e@gmx.net
<mailto:peter_e@gmx.net>>:

On 1/27/16 7:02 AM, Pavel Stehule wrote:

The issues:

1. configure missing systemd integration test, compilation fails:

postmaster.o postmaster.c
postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
directory

Updated patch attached that fixes this by adding additional checking in
configure.

You sent only rebased code of previous version. I didn't find additional
checks.

Oops. Here is the actual new code.

Attachments:

0001-Improve-error-reporting-when-location-specified-by-p.patchapplication/x-patch; name=0001-Improve-error-reporting-when-location-specified-by-p.patchDownload
From 607323c95ca62d74668992219569c7cff470b63d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:47:18 -0500
Subject: [PATCH 1/3] Improve error reporting when location specified by
 postgres -D does not exist

Previously, the first error seen would be that postgresql.conf does not
exist.  But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
---
 src/backend/utils/misc/guc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 38ba82f..b8d34b5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4463,6 +4463,17 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	else
 		configdir = make_absolute_path(getenv("PGDATA"));
 
+	if (configdir && stat(configdir, &stat_buf) != 0)
+	{
+		write_stderr("%s: could not access \"%s\": %s\n",
+					 progname,
+					 configdir,
+					 strerror(errno));
+		if (errno == ENOENT)
+			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		return false;
+	}
+
 	/*
 	 * Find the configuration file: if config_file was specified on the
 	 * command line, use it, else use configdir/postgresql.conf.  In any case
@@ -4498,7 +4509,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 */
 	if (stat(ConfigFileName, &stat_buf) != 0)
 	{
-		write_stderr("%s cannot access the server configuration file \"%s\": %s\n",
+		write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
 					 progname, ConfigFileName, strerror(errno));
 		free(configdir);
 		return false;
-- 
2.7.0

0002-Add-support-for-systemd-service-notifications.patchapplication/x-patch; name=0002-Add-support-for-systemd-service-notifications.patchDownload
From 8a2156bd3add4c8427d216df5757554a421573e7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 17 Nov 2015 06:46:17 -0500
Subject: [PATCH 2/3] Add support for systemd service notifications

Insert sd_notify() calls at server start and stop for integration with
systemd.  This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
---
 configure                           | 49 +++++++++++++++++++++++++++++++++++++
 configure.in                        | 13 ++++++++++
 doc/src/sgml/installation.sgml      | 16 ++++++++++++
 doc/src/sgml/runtime.sgml           | 24 ++++++++++++++++++
 src/Makefile.global.in              |  1 +
 src/backend/Makefile                |  4 +++
 src/backend/postmaster/postmaster.c | 21 ++++++++++++++++
 src/include/pg_config.h.in          |  3 +++
 8 files changed, 131 insertions(+)

diff --git a/configure b/configure
index 3dd1b15..b3f3abe 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ with_libxml
 XML2_CONFIG
 UUID_EXTRA_OBJS
 with_uuid
+with_systemd
 with_selinux
 with_openssl
 krb_srvtab
@@ -830,6 +831,7 @@ with_ldap
 with_bonjour
 with_openssl
 with_selinux
+with_systemd
 with_readline
 with_libedit_preferred
 with_uuid
@@ -1518,6 +1520,7 @@ Optional Packages:
   --with-bonjour          build with Bonjour support
   --with-openssl          build with OpenSSL support
   --with-selinux          build with SELinux support
+  --with-systemd          build with systemd support
   --without-readline      do not use GNU Readline nor BSD Libedit for editing
   --with-libedit-preferred
                           prefer BSD Libedit over GNU Readline
@@ -5695,6 +5698,41 @@ fi
 $as_echo "$with_selinux" >&6; }
 
 #
+# Systemd
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with systemd support" >&5
+$as_echo_n "checking whether to build with systemd support... " >&6; }
+
+
+
+# Check whether --with-systemd was given.
+if test "${with_systemd+set}" = set; then :
+  withval=$with_systemd;
+  case $withval in
+    yes)
+
+$as_echo "#define USE_SYSTEMD 1" >>confdefs.h
+
+      ;;
+    no)
+      :
+      ;;
+    *)
+      as_fn_error $? "no argument expected for --with-systemd option" "$LINENO" 5
+      ;;
+  esac
+
+else
+  with_systemd=no
+
+fi
+
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_systemd" >&5
+$as_echo "$with_systemd" >&6; }
+
+#
 # Readline
 #
 
@@ -10475,6 +10513,17 @@ done
 
 fi
 
+if test "$with_systemd" = yes ; then
+  ac_fn_c_check_header_mongrel "$LINENO" "systemd/sd-daemon.h" "ac_cv_header_systemd_sd_daemon_h" "$ac_includes_default"
+if test "x$ac_cv_header_systemd_sd_daemon_h" = xyes; then :
+
+else
+  as_fn_error $? "header file <systemd/sd-daemon.h> is required for systemd support" "$LINENO" 5
+fi
+
+
+fi
+
 if test "$with_libxml" = yes ; then
   ac_fn_c_check_header_mongrel "$LINENO" "libxml/parser.h" "ac_cv_header_libxml_parser_h" "$ac_includes_default"
 if test "x$ac_cv_header_libxml_parser_h" = xyes; then :
diff --git a/configure.in b/configure.in
index 9398482..0bd90d7 100644
--- a/configure.in
+++ b/configure.in
@@ -700,6 +700,15 @@ AC_SUBST(with_selinux)
 AC_MSG_RESULT([$with_selinux])
 
 #
+# Systemd
+#
+AC_MSG_CHECKING([whether to build with systemd support])
+PGAC_ARG_BOOL(with, systemd, no, [build with systemd support],
+              [AC_DEFINE([USE_SYSTEMD], 1, [Define to build with systemd support. (--with-systemd)])])
+AC_SUBST(with_systemd)
+AC_MSG_RESULT([$with_systemd])
+
+#
 # Readline
 #
 PGAC_ARG_BOOL(with, readline, yes,
@@ -1249,6 +1258,10 @@ if test "$with_pam" = yes ; then
                                      [AC_MSG_ERROR([header file <security/pam_appl.h> or <pam/pam_appl.h> is required for PAM.])])])
 fi
 
+if test "$with_systemd" = yes ; then
+  AC_CHECK_HEADER(systemd/sd-daemon.h, [], [AC_MSG_ERROR([header file <systemd/sd-daemon.h> is required for systemd support])])
+fi
+
 if test "$with_libxml" = yes ; then
   AC_CHECK_HEADER(libxml/parser.h, [], [AC_MSG_ERROR([header file <libxml/parser.h> is required for XML support])])
 fi
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 8964834..7b6a389 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -813,6 +813,22 @@ <title>Configuration</title>
       </varlistentry>
 
       <varlistentry>
+       <term><option>--with-systemd</option></term>
+       <listitem>
+        <para>
+         Build with support
+         for <application>systemd</application><indexterm><primary>systemd</primary></indexterm>
+         service notifications.  This improves integration if the server binary
+         is started under <application>systemd</application> but has no impact
+         otherwise; see <xref linkend="server-start"> for more
+         information.  <application>libsystemd</application> and the
+         associated header files need to be installed to be able to use this
+         option.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term><option>--without-readline</option></term>
        <listitem>
         <para>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index cda05f5..58b8838 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -369,6 +369,30 @@ <title>Starting the Database Server</title>
       <filename>contrib/start-scripts/linux</filename> in the
       <productname>PostgreSQL</productname> source distribution.
      </para>
+
+     <para>
+      When using <application>systemd</application>, you can use the following
+      service unit file (e.g.,
+      at <filename>/etc/systemd/system/postgresql.service</filename>):<indexterm><primary>systemd</primary></indexterm>
+<programlisting>
+[Unit]
+Description=PostgreSQL database server
+Documentation=man:postgres(1)
+
+[Service]
+Type=notify
+User=postgres
+ExecStart=/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data
+ExecReload=/bin/kill -HUP $MAINPID
+KillMode=mixed
+KillSignal=SIGINT
+
+[Install]
+WantedBy=multi-user.target
+</programlisting>
+      Using <literal>Type=notify</literal> requires that the server binary was
+      built with <literal>configure --with-systemd</literal>.
+     </para>
     </listitem>
 
     <listitem>
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 51f4797..e94d6a5 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -184,6 +184,7 @@ with_python	= @with_python@
 with_tcl	= @with_tcl@
 with_openssl	= @with_openssl@
 with_selinux	= @with_selinux@
+with_systemd	= @with_systemd@
 with_libxml	= @with_libxml@
 with_libxslt	= @with_libxslt@
 with_system_tzdata = @with_system_tzdata@
diff --git a/src/backend/Makefile b/src/backend/Makefile
index d4db8ff..b3d5e2e 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -45,6 +45,10 @@ LIBS := $(filter-out -lpgport -lpgcommon, $(LIBS)) $(LDAP_LIBS_BE)
 # The backend doesn't need everything that's in LIBS, however
 LIBS := $(filter-out -lz -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
 
+ifeq ($(with_systemd),yes)
+LIBS += -lsystemd
+endif
+
 ##########################################################################
 
 all: submake-libpgport submake-schemapg postgres $(POSTGRES_IMP)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9aaed5b..2e7f1d7 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -87,6 +87,10 @@
 #include <dns_sd.h>
 #endif
 
+#ifdef USE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
+
 #ifdef HAVE_PTHREAD_IS_THREADED_NP
 #include <pthread.h>
 #endif
@@ -2533,6 +2537,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = SmartShutdown;
 			ereport(LOG,
 					(errmsg("received smart shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			if (pmState == PM_RUN || pmState == PM_RECOVERY ||
 				pmState == PM_HOT_STANDBY || pmState == PM_STARTUP)
@@ -2585,6 +2592,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = FastShutdown;
 			ereport(LOG,
 					(errmsg("received fast shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			if (StartupPID != 0)
 				signal_child(StartupPID, SIGTERM);
@@ -2645,6 +2655,9 @@ pmdie(SIGNAL_ARGS)
 			Shutdown = ImmediateShutdown;
 			ereport(LOG,
 					(errmsg("received immediate shutdown request")));
+#ifdef USE_SYSTEMD
+			sd_notify(0, "STOPPING=1");
+#endif
 
 			TerminateChildren(SIGQUIT);
 			pmState = PM_WAIT_BACKENDS;
@@ -2787,6 +2800,10 @@ reaper(SIGNAL_ARGS)
 			ereport(LOG,
 				 (errmsg("database system is ready to accept connections")));
 
+#ifdef USE_SYSTEMD
+			sd_notify(0, "READY=1");
+#endif
+
 			continue;
 		}
 
@@ -4930,6 +4947,10 @@ sigusr1_handler(SIGNAL_ARGS)
 		ereport(LOG,
 		(errmsg("database system is ready to accept read only connections")));
 
+#ifdef USE_SYSTEMD
+		sd_notify(0, "READY=1");
+#endif
+
 		pmState = PM_HOT_STANDBY;
 		/* Some workers may be scheduled to start now */
 		StartWorkerNeeded = true;
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 16a272e..b3ceea5 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -833,6 +833,9 @@
 /* Define to 1 to use Intel SSSE 4.2 CRC instructions with a runtime check. */
 #undef USE_SSE42_CRC32C_WITH_RUNTIME_CHECK
 
+/* Define to build with systemd support. (--with-systemd) */
+#undef USE_SYSTEMD
+
 /* Define to select SysV-style semaphores. */
 #undef USE_SYSV_SEMAPHORES
 
-- 
2.7.0

#11Christoph Berg
myon@debian.org
In reply to: Peter Eisentraut (#10)
Re: [PATCH] better systemd integration

Hi Peter,

thanks for working on this, I'm looking forward to make Debian's
pg_*cluster tools work with that (and hopefully be able to remove tons
of legacy code).

If a cluster is configured for non-hot-standby replication, the
READY=1 seems to never happen. Did you check if that doesn't trigger
any timeouts with would make the unit "fail" or the like?

@@ -2787,6 +2800,10 @@ reaper(SIGNAL_ARGS)
ereport(LOG,
(errmsg("database system is ready to accept connections")));

+#ifdef USE_SYSTEMD
+                       sd_notify(0, "READY=1");
+#endif
+
                        continue;
                }

@@ -4930,6 +4947,10 @@ sigusr1_handler(SIGNAL_ARGS)
ereport(LOG,
(errmsg("database system is ready to accept read only connections")));

+#ifdef USE_SYSTEMD
+               sd_notify(0, "READY=1");
+#endif
+
                pmState = PM_HOT_STANDBY;
                /* Some workers may be scheduled to start now */
                StartWorkerNeeded = true;

Also, I'm wondering how hard it would be to get socket activation work
with that? (I wouldn't necessarily recommend that for production use,
but on my desktop it would certainly be helpful not to have all those
8.4/9.0/.../9.6 clusters running all the time doing nothing.)

Christoph
--
cb@df7cb.de | http://www.df7cb.de/

#12Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#10)
Re: [PATCH] better systemd integration

I wonder if instead of HAVE_SYSTEMD at each callsite we shouldn't
instead have a pg_sd_notify() call that's a no-op when not systemd.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#10)
Re: [PATCH] better systemd integration

Hi

You sent only rebased code of previous version. I didn't find additional
checks.

Oops. Here is the actual new code.

New test is working as expected

I did lot of tests - and this code works perfect in single server mode, and
with slave hot-standby mode.

It doesn't work with only standby mode

[root@dhcppc1 pavel]# systemctl start pg2.service
Job for pg2.service failed because a timeout was exceeded. See "systemctl
status pg2.service" and "journalctl -xe" for details.

Default timeout on FC is 90 sec - it is should not to be enough for large
servers with large shared buffers and high checkpoint segments. It should
be mentioned in service file.

Regards

Pavel

#14Peter Eisentraut
peter_e@gmx.net
In reply to: Christoph Berg (#11)
Re: [PATCH] better systemd integration

On 1/28/16 9:46 AM, Christoph Berg wrote:

If a cluster is configured for non-hot-standby replication, the
READY=1 seems to never happen. Did you check if that doesn't trigger
any timeouts with would make the unit "fail" or the like?

As Pavel showed, it doesn't work for that. I'll look into that.

Also, I'm wondering how hard it would be to get socket activation work
with that? (I wouldn't necessarily recommend that for production use,
but on my desktop it would certainly be helpful not to have all those
8.4/9.0/.../9.6 clusters running all the time doing nothing.)

I had looked into socket activation, and it looks feasible, but it's a
separate feature. I couldn't really think of a strong use case, but
what you describe makes sense.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Peter Eisentraut
peter_e@gmx.net
In reply to: Alvaro Herrera (#12)
Re: [PATCH] better systemd integration

On 1/28/16 10:08 AM, Alvaro Herrera wrote:

I wonder if instead of HAVE_SYSTEMD at each callsite we shouldn't
instead have a pg_sd_notify() call that's a no-op when not systemd.

We do this for other optional features as well, and I think it keeps the
code clearest, especially if the ifdef'ed sections are short.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#13)
Re: [PATCH] better systemd integration

On 1/29/16 4:15 PM, Pavel Stehule wrote:

Hi

You sent only rebased code of previous version. I didn't find additional
checks.

Oops. Here is the actual new code.

New test is working as expected

I did lot of tests - and this code works perfect in single server mode,
and with slave hot-standby mode.

It doesn't work with only standby mode

Yeah, I hadn't though of that. How about this change in addition:

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 2e7f1d7..d983a50 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4933,6 +4933,11 @@ sigusr1_handler(SIGNAL_ARGS)
        if (XLogArchivingAlways())
            PgArchPID = pgarch_start();
+#ifdef USE_SYSTEMD
+       if (!EnableHotStandby)
+           sd_notify(0, "READY=1");
+#endif
+
        pmState = PM_RECOVERY;
    }
    if (CheckPostmasterSignal(PMSIGNAL_BEGIN_HOT_STANDBY) &&

Default timeout on FC is 90 sec - it is should not to be enough for
large servers with large shared buffers and high checkpoint segments. It
should be mentioned in service file.

Good point. I think we should set TimeoutSec=0 in the suggested service file.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#16)
Re: [PATCH] better systemd integration

2016-01-30 22:38 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:

On 1/29/16 4:15 PM, Pavel Stehule wrote:

Hi

You sent only rebased code of previous version. I didn't find

additional

checks.

Oops. Here is the actual new code.

New test is working as expected

I did lot of tests - and this code works perfect in single server mode,
and with slave hot-standby mode.

It doesn't work with only standby mode

Yeah, I hadn't though of that. How about this change in addition:

diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 2e7f1d7..d983a50 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4933,6 +4933,11 @@ sigusr1_handler(SIGNAL_ARGS)
if (XLogArchivingAlways())
PgArchPID = pgarch_start();
+#ifdef USE_SYSTEMD
+       if (!EnableHotStandby)
+           sd_notify(0, "READY=1");
+#endif
+
pmState = PM_RECOVERY;
}
if (CheckPostmasterSignal(PMSIGNAL_BEGIN_HOT_STANDBY) &&

Default timeout on FC is 90 sec - it is should not to be enough for
large servers with large shared buffers and high checkpoint segments. It
should be mentioned in service file.

Good point. I think we should set TimeoutSec=0 in the suggested service
file.

probably no other is safe

Pavel

#18Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#16)
Re: [PATCH] better systemd integration

Hi

index 2e7f1d7..d983a50 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4933,6 +4933,11 @@ sigusr1_handler(SIGNAL_ARGS)
if (XLogArchivingAlways())
PgArchPID = pgarch_start();
+#ifdef USE_SYSTEMD
+       if (!EnableHotStandby)
+           sd_notify(0, "READY=1");
+#endif
+
pmState = PM_RECOVERY;
}
if (CheckPostmasterSignal(PMSIGNAL_BEGIN_HOT_STANDBY) &&

I cannot to check it this week, but this should to work. "ready" state for
standby only mode starting when slave is able to get wal records or
segments from server. I will test it next week.

regards

Pavel

Default timeout on FC is 90 sec - it is should not to be enough for
large servers with large shared buffers and high checkpoint segments. It
should be mentioned in service file.

Good point. I think we should set TimeoutSec=0 in the suggested service

file.

Show quoted text
#19Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#18)
Re: [PATCH] better systemd integration

I've committed this. Thanks for checking.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers