From 39bd025c295974986331ca4cd89ce18cfa50204a Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Mar 2012 11:47:51 +0200 Subject: [PATCH 1/5] Redis software watchdog. --- src/config.c | 7 ++++++ src/debug.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++-- src/redis.c | 7 +++++- src/redis.h | 4 ++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/config.c b/src/config.c index ab49178a..8dffe288 100644 --- a/src/config.c +++ b/src/config.c @@ -627,6 +627,12 @@ void configSetCommand(redisClient *c) { } else if (!strcasecmp(c->argv[2]->ptr,"repl-timeout")) { if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll <= 0) goto badfmt; server.repl_timeout = ll; + } else if (!strcasecmp(c->argv[2]->ptr,"watchdog-period")) { + if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll < 0) goto badfmt; + if (ll) + enableWatchdog(ll); + else + disableWatchdog(); } else { addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s", (char*)c->argv[2]->ptr); @@ -715,6 +721,7 @@ void configGetCommand(redisClient *c) { config_get_numerical_field("repl-ping-slave-period",server.repl_ping_slave_period); config_get_numerical_field("repl-timeout",server.repl_timeout); config_get_numerical_field("maxclients",server.maxclients); + config_get_numerical_field("watchdog-period",server.watchdog_period); /* Bool (yes/no) values */ config_get_bool_field("no-appendfsync-on-rewrite", diff --git a/src/debug.c b/src/debug.c index 9c8b6ef9..2df913b0 100644 --- a/src/debug.c +++ b/src/debug.c @@ -661,11 +661,72 @@ void sigsegvHandler(int sig, siginfo_t *info, void *secret) { /* Make sure we exit with the right signal at the end. So for instance * the core will be dumped if enabled. */ sigemptyset (&act.sa_mask); - /* When the SA_SIGINFO flag is set in sa_flags then sa_sigaction - * is used. Otherwise, sa_handler is used */ act.sa_flags = SA_NODEFER | SA_ONSTACK | SA_RESETHAND; act.sa_handler = SIG_DFL; sigaction (sig, &act, NULL); kill(getpid(),sig); } #endif /* HAVE_BACKTRACE */ + +/* =========================== Software Watchdog ============================ */ +#include + +void watchdogSignalHandler(int sig, siginfo_t *info, void *secret) { + ucontext_t *uc = (ucontext_t*) secret; + REDIS_NOTUSED(info); + REDIS_NOTUSED(sig); + + /* Log INFO and CLIENT LIST */ + redisLog(REDIS_WARNING, "--- WATCHDOG TIMER EXPIRED ---"); +#ifdef HAVE_BACKTRACE + logStackTrace(uc); + redisLog(REDIS_WARNING, "------"); +#endif +} + +/* Schedule a SIGALRM delivery after the specified period in milliseconds. + * If a timer is already scheduled, this function will re-schedule it to the + * specified time. If period is 0 the current timer is disabled. */ +void watchdogScheduleSignal(int period) { + struct itimerval it; + + /* Will stop the timer if period is 0. */ + it.it_value.tv_sec = period/1000; + it.it_value.tv_usec = period%1000; + /* Don't automatically restart. */ + it.it_interval.tv_sec = 0; + it.it_interval.tv_usec = 0; + setitimer(ITIMER_REAL, &it, NULL); +} + +/* Enable the software watchdong with the specified period in milliseconds. */ +void enableWatchdog(int period) { + if (server.watchdog_period == 0) { + struct sigaction act; + + /* Watchdog was actually disabled, so we have to setup the signal + * handler. */ + sigemptyset(&act.sa_mask); + act.sa_flags = SA_NODEFER | SA_ONSTACK | SA_SIGINFO; + act.sa_sigaction = watchdogSignalHandler; + sigaction(SIGALRM, &act, NULL); + } + if (period < 200) period = 200; /* We don't accept periods < 200 ms. */ + watchdogScheduleSignal(period); /* Adjust the current timer. */ + server.watchdog_period = period; +} + +/* Disable the software watchdog. */ +void disableWatchdog(void) { + struct sigaction act; + if (server.watchdog_period == 0) return; /* Already disabled. */ + watchdogScheduleSignal(0); /* Stop the current timer. */ + + /* Set the signal handler to SIG_IGN, this will also remove pending + * signals from the queue. */ + sigemptyset(&act.sa_mask); + act.sa_flags = 0; + act.sa_handler = SIG_IGN; + sigaction(SIGALRM, &act, NULL); + server.watchdog_period = 0; +} diff --git a/src/redis.c b/src/redis.c index e926fd9b..88fb2fd8 100644 --- a/src/redis.c +++ b/src/redis.c @@ -726,6 +726,10 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) { REDIS_NOTUSED(id); REDIS_NOTUSED(clientData); + /* Software watchdog: deliver the SIGALRM that will reach the signal + * handler if we don't return here fast enough. */ + if (server.watchdog_period) watchdogScheduleSignal(server.watchdog_period); + /* We take a cached value of the unix time in the global state because * with virtual memory and aging there is to store the current time * in objects at every object access, and accuracy is not needed. @@ -1086,11 +1090,12 @@ void initServerConfig() { server.slowlog_log_slower_than = REDIS_SLOWLOG_LOG_SLOWER_THAN; server.slowlog_max_len = REDIS_SLOWLOG_MAX_LEN; - /* Assert */ + /* Debugging */ server.assert_failed = ""; server.assert_file = ""; server.assert_line = 0; server.bug_report_start = 0; + server.watchdog_period = 0; } /* This function will try to raise the max number of open files accordingly to diff --git a/src/redis.h b/src/redis.h index e4fd47d3..20e0ea98 100644 --- a/src/redis.h +++ b/src/redis.h @@ -722,6 +722,7 @@ struct redisServer { char *assert_file; int assert_line; int bug_report_start; /* True if bug report header was already logged. */ + int watchdog_period; /* Software watchdog period in ms. 0 = off */ }; typedef struct pubsubPattern { @@ -1255,4 +1256,7 @@ void bugReportStart(void); void redisLogObjectDebugInfo(robj *o); void sigsegvHandler(int sig, siginfo_t *info, void *secret); sds genRedisInfoString(char *section); +void enableWatchdog(int period); +void disableWatchdog(void); +void watchdogScheduleSignal(int period); #endif From a354da9acd8afbe4e600360ef6502c10dfed3ccb Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Mar 2012 12:11:37 +0200 Subject: [PATCH 2/5] Correctly set the SIGARLM timer for the software watchdog. --- src/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/debug.c b/src/debug.c index 2df913b0..30fb0091 100644 --- a/src/debug.c +++ b/src/debug.c @@ -692,7 +692,7 @@ void watchdogScheduleSignal(int period) { /* Will stop the timer if period is 0. */ it.it_value.tv_sec = period/1000; - it.it_value.tv_usec = period%1000; + it.it_value.tv_usec = (period%1000)*1000; /* Don't automatically restart. */ it.it_interval.tv_sec = 0; it.it_interval.tv_usec = 0; From aa96122d968308f77c4d26ba24c6ec9727b4e88b Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Mar 2012 13:48:53 +0200 Subject: [PATCH 3/5] Mask SIGALRM everything but in the main thread. This is required to ensure that the signal will be delivered to the main thread when the watchdog timer expires. --- src/bio.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/bio.c b/src/bio.c index eaac8e40..aa2cdf9f 100644 --- a/src/bio.c +++ b/src/bio.c @@ -108,9 +108,18 @@ void bioCreateBackgroundJob(int type, void *arg1, void *arg2, void *arg3) { void *bioProcessBackgroundJobs(void *arg) { struct bio_job *job; unsigned long type = (unsigned long) arg; + sigset_t sigset; pthread_detach(pthread_self()); pthread_mutex_lock(&bio_mutex[type]); + /* Block SIGALRM so we are sure that only the main thread will + * receive the watchdog signal. */ + sigemptyset(&sigset); + sigaddset(&sigset, SIGALRM); + if (pthread_sigmask(SIG_BLOCK, &sigset, NULL)) + redisLog(REDIS_WARNING, + "Warning: can't mask SIGALRM in bio.c thread: %s", strerror(errno)); + while(1) { listNode *ln; From 23c0cdd2ad8b15defab56eca89a42c67cadd9a34 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 27 Mar 2012 15:24:33 +0200 Subject: [PATCH 4/5] Produce the watchlog warning log in a way that is safer from a signal handler. Fix a memory leak in the backtrace generation function. --- src/debug.c | 41 +++++++++++++++++++++++++++++++---------- src/zmalloc.c | 4 ++++ src/zmalloc.h | 1 + 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/debug.c b/src/debug.c index 30fb0091..49c76824 100644 --- a/src/debug.c +++ b/src/debug.c @@ -559,10 +559,11 @@ void logRegisters(ucontext_t *uc) { } /* Logs the stack trace using the backtrace() call. */ -void logStackTrace(ucontext_t *uc) { +sds getStackTrace(ucontext_t *uc) { void *trace[100]; int i, trace_size = 0; char **messages = NULL; + sds st = sdsempty(); /* Generate the stack trace */ trace_size = backtrace(trace, 100); @@ -572,9 +573,12 @@ void logStackTrace(ucontext_t *uc) { trace[1] = getMcontextEip(uc); } messages = backtrace_symbols(trace, trace_size); - redisLog(REDIS_WARNING, "--- STACK TRACE"); - for (i=1; i Date: Tue, 27 Mar 2012 16:54:53 +0200 Subject: [PATCH 5/5] define zlibc_free() in a way that is not shadowed by jemalloc. --- src/zmalloc.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/zmalloc.c b/src/zmalloc.c index 5e5598cf..79b56158 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -30,6 +30,15 @@ #include #include + +/* This function provide us access to the original libc free(). This is useful + * for instance to free results obtained by backtrace_symbols(). We need + * to define this function before including zmalloc.h that may shadow the + * free implementation if we use jemalloc or another non standard allocator. */ +void zlibc_free(void *ptr) { + free(ptr); +} + #include #include #include "config.h" @@ -227,10 +236,6 @@ void zmalloc_enable_thread_safeness(void) { zmalloc_thread_safe = 1; } -void zlibc_free(void *ptr) { - free(ptr); -} - /* Get the RSS information in an OS-specific way. * * WARNING: the function zmalloc_get_rss() is not designed to be fast