From 641c64ada10404356fc76c0b56a69b32c76f253c Mon Sep 17 00:00:00 2001 From: Johannes Truschnigg Date: Sat, 27 Apr 2019 18:07:30 +0200 Subject: [PATCH 1/5] Use libsystemd's sd_notify for communicating redis status to systemd Instead of replicating a subset of libsystemd's sd_notify(3) internally, use the dynamic library provided by systemd to communicate with the service manager. When systemd supervision was auto-detected or configured, communicate the actual server status (i.e. "Loading dataset", "Waiting for master<->replica sync") to systemd, instead of declaring readiness right after initializing the server process. --- src/replication.c | 5 +++ src/server.c | 90 +++++++++++++++++++++++------------------------ src/server.h | 1 + 3 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/replication.c b/src/replication.c index c9a2e0fe1..68dc77a61 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1665,6 +1665,11 @@ void readSyncBulkPayload(connection *conn) { if (server.repl_backlog == NULL) createReplicationBacklog(); serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: Finished with success"); + if (server.supervised_mode == SUPERVISED_SYSTEMD) { + redisCommunicateSystemd("STATUS=MASTER <-> REPLICA sync: Finished with success. Ready to accept connections.\n"); + redisCommunicateSystemd("READY=1\n"); + } + /* Restart the AOF subsystem now that we finished the sync. This * will trigger an AOF rewrite, and when done will start appending * to the new file. */ diff --git a/src/server.c b/src/server.c index 2e9a329dd..e86f16aa8 100644 --- a/src/server.c +++ b/src/server.c @@ -55,6 +55,7 @@ #include #include #include +#include /* Our shared "common" objects */ @@ -3651,6 +3652,8 @@ int prepareForShutdown(int flags) { int nosave = flags & SHUTDOWN_NOSAVE; serverLog(LL_WARNING,"User requested shutdown..."); + if (server.supervised_mode == SUPERVISED_SYSTEMD) + redisCommunicateSystemd("STOPPING=1\n"); /* Kill all the Lua debugger forked sessions. */ ldbKillForkedSessions(); @@ -3692,6 +3695,8 @@ int prepareForShutdown(int flags) { /* Create a new RDB file before exiting. */ if ((server.saveparamslen > 0 && !nosave) || save) { serverLog(LL_NOTICE,"Saving the final RDB snapshot before exiting."); + if (server.supervised_mode == SUPERVISED_SYSTEMD) + redisCommunicateSystemd("STATUS=Saving the final RDB snapshot\n"); /* Snapshotting. Perform a SYNC SAVE and exit */ rdbSaveInfo rsi, *rsiptr; rsiptr = rdbPopulateSaveInfo(&rsi); @@ -3702,6 +3707,8 @@ int prepareForShutdown(int flags) { * saving aborted, handling special stuff like slaves pending for * synchronization... */ serverLog(LL_WARNING,"Error trying to save the DB, can't exit."); + if (server.supervised_mode == SUPERVISED_SYSTEMD) + redisCommunicateSystemd("STATUS=Error trying to save the DB, can't exit.\n"); return C_ERR; } } @@ -4867,13 +4874,11 @@ int redisSupervisedUpstart(void) { return 1; } -int redisSupervisedSystemd(void) { +int redisCommunicateSystemd(const char *sd_notify_msg) { const char *notify_socket = getenv("NOTIFY_SOCKET"); - int fd = 1; - struct sockaddr_un su; - struct iovec iov; - struct msghdr hdr; - int sendto_flags = 0; + int (*dl_sd_notify)(int unset_environment, const char *state); + char *error; + void *handle; if (!notify_socket) { serverLog(LL_WARNING, @@ -4881,47 +4886,28 @@ int redisSupervisedSystemd(void) { return 0; } - if ((strchr("@/", notify_socket[0])) == NULL || strlen(notify_socket) < 2) { - return 0; - } - - serverLog(LL_NOTICE, "supervised by systemd, will signal readiness"); - if ((fd = socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) { + handle = dlopen("libsystemd.so.0", RTLD_LAZY); + if (!handle) { serverLog(LL_WARNING, - "Can't connect to systemd socket %s", notify_socket); + "systemd supervision requested, but could not dlopen() libsystemd.so"); + (void) dlerror(); + return 0; + } + (void) dlerror(); + + *(void **)(&dl_sd_notify) = dlsym(handle, "sd_notify"); + error = dlerror(); + + if (error != NULL) { + serverLog(LL_WARNING, + "systemd supervision requested, but could not load sd_notify(3) from libsystemd.so"); + dlclose(handle); return 0; } - memset(&su, 0, sizeof(su)); - su.sun_family = AF_UNIX; - strncpy (su.sun_path, notify_socket, sizeof(su.sun_path) -1); - su.sun_path[sizeof(su.sun_path) - 1] = '\0'; - - if (notify_socket[0] == '@') - su.sun_path[0] = '\0'; - - memset(&iov, 0, sizeof(iov)); - iov.iov_base = "READY=1"; - iov.iov_len = strlen("READY=1"); - - memset(&hdr, 0, sizeof(hdr)); - hdr.msg_name = &su; - hdr.msg_namelen = offsetof(struct sockaddr_un, sun_path) + - strlen(notify_socket); - hdr.msg_iov = &iov; - hdr.msg_iovlen = 1; - - unsetenv("NOTIFY_SOCKET"); -#ifdef HAVE_MSG_NOSIGNAL - sendto_flags |= MSG_NOSIGNAL; -#endif - if (sendmsg(fd, &hdr, sendto_flags) < 0) { - serverLog(LL_WARNING, "Can't send notification to systemd"); - close(fd); - return 0; - } - close(fd); - return 1; + (void) (*dl_sd_notify)(0, sd_notify_msg); + dlclose(handle); + return 0; } int redisIsSupervised(int mode) { @@ -4932,12 +4918,17 @@ int redisIsSupervised(int mode) { if (upstart_job) { redisSupervisedUpstart(); } else if (notify_socket) { - redisSupervisedSystemd(); + server.supervised_mode = SUPERVISED_SYSTEMD; + serverLog(LL_WARNING, + "WARNING auto-supervised by systemd - you MUST set appropriate values for TimeoutStartSec and TimeoutStopSec in your service unit."); + return redisCommunicateSystemd("STATUS=Redis is loading...\n"); } } else if (mode == SUPERVISED_UPSTART) { return redisSupervisedUpstart(); } else if (mode == SUPERVISED_SYSTEMD) { - return redisSupervisedSystemd(); + serverLog(LL_WARNING, + "WARNING supervised by systemd - you MUST set appropriate values for TimeoutStartSec and TimeoutStopSec in your service unit."); + return redisCommunicateSystemd("STATUS=Redis is loading...\n"); } return 0; @@ -5130,6 +5121,15 @@ int main(int argc, char **argv) { serverLog(LL_NOTICE,"Ready to accept connections"); if (server.sofd > 0) serverLog(LL_NOTICE,"The server is now ready to accept connections at %s", server.unixsocket); + if (server.supervised_mode == SUPERVISED_SYSTEMD) { + /* XXX TODO is this sufficient to pass readiness control off to readSyncBulkPayload()? */ + if (!server.masterhost) { + redisCommunicateSystemd("STATUS=Ready to accept connections\n"); + redisCommunicateSystemd("READY=1\n"); + } else { + redisCommunicateSystemd("STATUS=Waiting for MASTER <-> REPLICA sync\n"); + } + } } else { InitServerLast(); sentinelIsRunning(); diff --git a/src/server.h b/src/server.h index ad47fe80b..28aa2085c 100644 --- a/src/server.h +++ b/src/server.h @@ -1620,6 +1620,7 @@ uint64_t crc64(uint64_t crc, const unsigned char *s, uint64_t l); void exitFromChild(int retcode); size_t redisPopcount(void *s, long count); void redisSetProcTitle(char *title); +int redisCommunicateSystemd(const char *sd_notify_msg); /* networking.c -- Networking and Client related operations */ client *createClient(connection *conn); From 5bbc112fb183a4e9b38ebf86535a52784d5b03ff Mon Sep 17 00:00:00 2001 From: Johannes Truschnigg Date: Sat, 27 Apr 2019 18:14:59 +0200 Subject: [PATCH 2/5] Provide example systemd service unit files for redis-server --- utils/systemd-redis_multiple_servers@.service | 37 +++++++++++++++++ utils/systemd-redis_server.service | 41 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 utils/systemd-redis_multiple_servers@.service create mode 100644 utils/systemd-redis_server.service diff --git a/utils/systemd-redis_multiple_servers@.service b/utils/systemd-redis_multiple_servers@.service new file mode 100644 index 000000000..108ccfc64 --- /dev/null +++ b/utils/systemd-redis_multiple_servers@.service @@ -0,0 +1,37 @@ +# example systemd template service unit file for multiple redis-servers +# +# You can use this file as a blueprint for your actual template service unit +# file, if you intend to run multiple independent redis-server instances in +# parallel using systemd's "template unit files" feature. If you do, you will +# want to choose a better basename for your service unit by renaming this file +# when copying it. +# +# Please take a look at the provided "systemd-redis_server.service" example +# service unit file, too, if you choose to use this approach at managing +# multiple redis-server instances via systemd. + +[Unit] +Description=Redis data structure server - instance %i +Documentation=https://redis.io/documentation +# This template unit assumes your redis-server configuration file(s) +# to live at /etc/redis/redis_server_.conf +AssertPathExists=/etc/redis/redis_server_%i.conf +#Before=your_application.service another_example_application.service +#AssertPathExists=/var/lib/redis + +[Service] +ExecStart=/usr/local/bin/redis-server /etc/redis/redis_server_%i.conf +LimitNOFILE=10032 +NoNewPrivileges=yes +#OOMScoreAdjust=-900 +#PrivateTmp=yes +Type=notify +TimeoutStartSec=infinity +TimeoutStopSec=infinity +UMask=0077 +#User=redis +#Group=redis +#WorkingDirectory=/var/lib/redis + +[Install] +WantedBy=multi-user.target diff --git a/utils/systemd-redis_server.service b/utils/systemd-redis_server.service new file mode 100644 index 000000000..addee3498 --- /dev/null +++ b/utils/systemd-redis_server.service @@ -0,0 +1,41 @@ +# example systemd service unit file for redis-server +# +# In order to use this as a template for providing a redis service in your +# environment, _at the very least_ make sure to adapt the redis configuration +# file you intend to use as needed (make sure to set "supervised systemd"), and +# to set sane TimeoutStartSec and TimeoutStopSec property values in the unit's +# "[Service]" section to fit your needs. +# +# Some properties, such as User= and Group=, are highly desirable for virtually +# all deployments of redis, but cannot be provided in a manner that fits all +# expectable environments. Some of these properties have been commented out in +# this example service unit file, but you are highly encouraged to set them to +# fit your needs. +# +# Please refer to systemd.unit(5), systemd.service(5), and systemd.exec(5) for +# more information. + +[Unit] +Description=Redis data structure server +Documentation=https://redis.io/documentation +#Before=your_application.service another_example_application.service +#AssertPathExists=/var/lib/redis + +[Service] +ExecStart=/usr/local/bin/redis-server --supervised systemd --daemonize no +## Alternatively, have redis-server load a configuration file: +#ExecStart=/usr/local/bin/redis-server /path/to/your/redis.conf +LimitNOFILE=10032 +NoNewPrivileges=yes +#OOMScoreAdjust=-900 +#PrivateTmp=yes +Type=notify +TimeoutStartSec=infinity +TimeoutStopSec=infinity +UMask=0077 +#User=redis +#Group=redis +#WorkingDirectory=/var/lib/redis + +[Install] +WantedBy=multi-user.target From ec5681f0f1e6df237c6b5e650317d52183bc19b7 Mon Sep 17 00:00:00 2001 From: Johannes Truschnigg Date: Sat, 27 Apr 2019 18:13:44 +0200 Subject: [PATCH 3/5] Do not install SysV init-scripts on systemd-enabled hosts Also, hint at example service unit files if systemd is detected. Thanks to @mika for spotting a bug in the original iteration of this patch. --- utils/install_server.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/utils/install_server.sh b/utils/install_server.sh index 8e5753bc6..efda7da1c 100755 --- a/utils/install_server.sh +++ b/utils/install_server.sh @@ -73,6 +73,16 @@ if [ "$(id -u)" -ne 0 ] ; then exit 1 fi +#bail if this system is managed by systemd +_pid_1_exe="$(readlink -f /proc/1/exe)" +if [ "${_pid_1_exe##*/}" = systemd ] +then + echo "This systems seems to use systemd." + echo "Please take a look at the provided example service unit files in this directory, and adapt and install them. Sorry!" + exit 1 +fi +unset _pid_1_exe + if ! echo $REDIS_PORT | egrep -q '^[0-9]+$' ; then _MANUAL_EXECUTION=true #Read the redis port From 129d14e1431e913426485526663e1a9aac67838c Mon Sep 17 00:00:00 2001 From: Johannes Truschnigg Date: Thu, 30 May 2019 18:44:17 +0200 Subject: [PATCH 4/5] Auto-detect and link libsystemd at compile-time This adds Makefile/build-system support for USE_SYSTEMD=(yes|no|*). This variable's value determines whether or not libsystemd will be linked at build-time. If USE_SYSTEMD is set to "yes", make will use PKG_CONFIG to check for libsystemd's presence, and fail the build early if it isn't installed/detected properly. If USE_SYSTEM is set to "no", libsystemd will *not* be linked, even if support for it is available on the system redis is being built on. For any other value that USE_SYSTEM might assume (e.g. "auto"), PKG_CONFIG will try to determine libsystemd's presence, and set up the build process to link against it, if it was indicated as being installed/available. This approach has a number of repercussions of its own, most importantly the following: If you build redis on a system that actually has systemd support, but no libsystemd-dev package(s) installed, you'll end up *without* support for systemd notification/status reporting support in redis-server. This changes established runtime behaviour. I'm not sure if the build system and/or the server binary should indicate this. I'm also wondering if not actually having systemd-notify-support, but requesting it via the server's config, should result in a fatal error now. --- src/Makefile | 25 +++++++++++++++++++++++++ src/server.c | 32 +++++--------------------------- src/server.h | 4 ++++ 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/Makefile b/src/Makefile index 9fc230f94..d9acdf57d 100644 --- a/src/Makefile +++ b/src/Makefile @@ -32,6 +32,7 @@ OPT=$(OPTIMIZATION) PREFIX?=/usr/local INSTALL_BIN=$(PREFIX)/bin INSTALL=install +PKG_CONFIG?=pkg-config # Default allocator defaults to Jemalloc if it's not an ARM MALLOC=libc @@ -131,6 +132,30 @@ endif # Include paths to dependencies FINAL_CFLAGS+= -I../deps/hiredis -I../deps/linenoise -I../deps/lua/src +# Determine systemd support and/or build preference (defaulting to auto-detection) +BUILD_WITH_SYSTEMD=no +# If 'USE_SYSTEMD' in the environment is neither "no" nor "yes", try to +# auto-detect libsystemd's presence and link accordingly. +ifneq ($(USE_SYSTEMD),no) + LIBSYSTEMD_PKGCONFIG := $(shell $(PKG_CONFIG) --exists libsystemd && echo $$?) +# If libsystemd cannot be detected, continue building without support for it +# (unless a later check tells us otherwise) +ifeq ($(LIBSYSTEMD_PKGCONFIG),0) + BUILD_WITH_SYSTEMD=yes +endif +endif +ifeq ($(USE_SYSTEMD),yes) +ifneq ($(LIBSYSTEMD_PKGCONFIG),0) +$(error USE_SYSTEMD is set to "$(USE_SYSTEMD)", but $(PKG_CONFIG) cannot find libsystemd) +endif +# Force building with libsystemd + BUILD_WITH_SYSTEMD=yes +endif +ifeq ($(BUILD_WITH_SYSTEMD),yes) + FINAL_LIBS+=$(shell $(PKG_CONFIG) --libs libsystemd) + FINAL_CFLAGS+= -DHAVE_LIBSYSTEMD +endif + ifeq ($(MALLOC),tcmalloc) FINAL_CFLAGS+= -DUSE_TCMALLOC FINAL_LIBS+= -ltcmalloc diff --git a/src/server.c b/src/server.c index e86f16aa8..664861b1f 100644 --- a/src/server.c +++ b/src/server.c @@ -55,7 +55,6 @@ #include #include #include -#include /* Our shared "common" objects */ @@ -4876,37 +4875,16 @@ int redisSupervisedUpstart(void) { int redisCommunicateSystemd(const char *sd_notify_msg) { const char *notify_socket = getenv("NOTIFY_SOCKET"); - int (*dl_sd_notify)(int unset_environment, const char *state); - char *error; - void *handle; - if (!notify_socket) { serverLog(LL_WARNING, "systemd supervision requested, but NOTIFY_SOCKET not found"); - return 0; } - handle = dlopen("libsystemd.so.0", RTLD_LAZY); - if (!handle) { - serverLog(LL_WARNING, - "systemd supervision requested, but could not dlopen() libsystemd.so"); - (void) dlerror(); - return 0; - } - (void) dlerror(); - - *(void **)(&dl_sd_notify) = dlsym(handle, "sd_notify"); - error = dlerror(); - - if (error != NULL) { - serverLog(LL_WARNING, - "systemd supervision requested, but could not load sd_notify(3) from libsystemd.so"); - dlclose(handle); - return 0; - } - - (void) (*dl_sd_notify)(0, sd_notify_msg); - dlclose(handle); + #ifdef HAVE_LIBSYSTEMD + (void) sd_notify(0, sd_notify_msg); + #else + UNUSED(sd_notify_msg); + #endif return 0; } diff --git a/src/server.h b/src/server.h index 28aa2085c..e96207c53 100644 --- a/src/server.h +++ b/src/server.h @@ -49,6 +49,10 @@ #include #include +#ifdef HAVE_LIBSYSTEMD +#include +#endif + typedef long long mstime_t; /* millisecond time type. */ typedef long long ustime_t; /* microsecond time type. */ From c7b68d10ea6137305715d2660897237a864c552e Mon Sep 17 00:00:00 2001 From: Maxim Ivanov Date: Wed, 20 Nov 2019 19:45:25 +0200 Subject: [PATCH 5/5] Prune leftover TODO comment Is it sufficient... ? -- Yes it is. In standalone mode, we say READY=1 at the comment point; however in replicated mode, we delay sending READY=1 until the replication sync completes. --- src/server.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server.c b/src/server.c index 664861b1f..dfecc0cdb 100644 --- a/src/server.c +++ b/src/server.c @@ -5100,7 +5100,6 @@ int main(int argc, char **argv) { if (server.sofd > 0) serverLog(LL_NOTICE,"The server is now ready to accept connections at %s", server.unixsocket); if (server.supervised_mode == SUPERVISED_SYSTEMD) { - /* XXX TODO is this sufficient to pass readiness control off to readSyncBulkPayload()? */ if (!server.masterhost) { redisCommunicateSystemd("STATUS=Ready to accept connections\n"); redisCommunicateSystemd("READY=1\n");