From aa0092c7d0244c0461bc2a13f62c0250ba7e43a8 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Aug 10 2012 20:44:15 +0000 Subject: sanlock: base kill sig on last renewal Tracking progression through the grace time by counting one retry per second doesn't work in the case were sanlock doesn't run every second (e.g. sigstop or delayed scheduling). This will cause sanlock to attempt to use killpath even when there's nearly no time left before reset. So, the transition to sigkill should depend on the current time from the last lease renewal. Signed-off-by: David Teigland --- diff --git a/src/cmd.c b/src/cmd.c index bc7d7da..c293275 100644 --- a/src/cmd.c +++ b/src/cmd.c @@ -1359,7 +1359,7 @@ static int print_state_daemon(char *str) "id_renewal=%d " "id_renewal_fail=%d " "id_renewal_warn=%d " - "kill_count_grace=%d " + "kill_grace_seconds=%d " "helper_pid=%d " "helper_kill_fd=%d " "helper_full_count=%u " @@ -1371,7 +1371,7 @@ static int print_state_daemon(char *str) main_task.id_renewal_seconds, main_task.id_renewal_fail_seconds, main_task.id_renewal_warn_seconds, - kill_count_grace, + kill_grace_seconds, helper_pid, helper_kill_fd, helper_full_count, diff --git a/src/main.c b/src/main.c index e5f0885..3ecc1f5 100644 --- a/src/main.c +++ b/src/main.c @@ -124,6 +124,11 @@ static void send_helper_kill(struct space *sp, struct client *cl, int sig) if ((cl->flags & CL_RUNPATH_SENT) && (sig == SIGRUNPATH)) return; + if (helper_kill_fd == -1) { + log_error("send_helper_kill pid %d no fd", cl->pid); + return; + } + memset(&hm, 0, sizeof(hm)); if (sig == SIGRUNPATH) { @@ -535,9 +540,9 @@ static int client_using_space(struct client *cl, struct space *sp) static void kill_pids(struct space *sp) { struct client *cl; - uint64_t now; + uint64_t now, last_success; int ci, fd, pid, sig; - int do_kill; + int do_kill, in_grace; /* * all remaining pids using sp are stuck, we've made max attempts to @@ -546,6 +551,17 @@ static void kill_pids(struct space *sp) if (sp->killing_pids > 1) return; + /* + * If we happen to renew our lease after we've started killing pids, + * the period we allow for graceful shutdown will be extended. This + * is an incidental effect, although it may be nice. The previous + * behavior would still be ok, where we only ever allow up to + * kill_grace_seconds for graceful shutdown before moving to sigkill. + */ + pthread_mutex_lock(&sp->mutex); + last_success = sp->lease_status.renewal_last_success; + pthread_mutex_unlock(&sp->mutex); + now = monotime(); for (ci = 0; ci <= client_maxi; ci++) { @@ -578,32 +594,33 @@ static void kill_pids(struct space *sp) fd = cl->fd; pid = cl->pid; - /* - * from zero to kill_count_grace seconds, we try killing - * the pid with either killpath or sigterm. killpath if - * it's configured and and we've seen a helper status recently. - * (sigkill will be used in place of sigterm if restricted.) - * - * after kill_count_grace seconds, we'll try killing the - * pid with sigkill. (sigterm will be used in place of - * sigkill if restricted.) + * the transition from using killpath/sigterm to sigkill + * is when now >= + * last successful lease renewal + + * id_renewal_fail_seconds + + * kill_grace_seconds */ - if (cl->killpath[0] && - (helper_kill_fd != -1) && - (kill_count_grace > 0) && - (cl->kill_count <= kill_count_grace) && - (now - helper_last_status < (HELPER_STATUS_INTERVAL * 2))) + in_grace = now < (last_success + main_task.id_renewal_fail_seconds + kill_grace_seconds); + + if ((kill_grace_seconds > 0) && in_grace && cl->killpath[0]) { sig = SIGRUNPATH; - else if (cl->restrict & SANLK_RESTRICT_SIGKILL) + } else if (in_grace) { sig = SIGTERM; - else if (cl->restrict & SANLK_RESTRICT_SIGTERM) + } else { sig = SIGKILL; - else if ((kill_count_grace > 0) && - (cl->kill_count <= kill_count_grace)) + } + + /* + * sigterm will be used in place of sigkill if restricted + * sigkill will be used in place of sigterm if restricted + */ + + if ((sig == SIGKILL) && (cl->restrict & SANLK_RESTRICT_SIGKILL)) sig = SIGTERM; - else + + if ((sig == SIGTERM) && (cl->restrict & SANLK_RESTRICT_SIGTERM)) sig = SIGKILL; do_kill = 1; @@ -1966,7 +1983,7 @@ static int read_command_line(int argc, char *argv[]) if (com.type == COM_DAEMON) { sec = atoi(optionarg); if (sec <= 60 && sec >= 0) - kill_count_grace = sec; + kill_grace_seconds = sec; } else { com.local_host_generation = atoll(optionarg); } @@ -2343,8 +2360,8 @@ int main(int argc, char *argv[]) /* initialize global EXTERN variables */ - kill_count_max = 60; - kill_count_grace = DEFAULT_GRACE_SEC; + kill_count_max = 100; + kill_grace_seconds = DEFAULT_GRACE_SEC; helper_ci = -1; helper_pid = -1; helper_kill_fd = -1; diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h index 9950ebd..9a30763 100644 --- a/src/sanlock_internal.h +++ b/src/sanlock_internal.h @@ -316,7 +316,7 @@ EXTERN int external_shutdown; EXTERN char our_host_name_global[SANLK_NAME_LEN+1]; EXTERN int kill_count_max; -EXTERN int kill_count_grace; +EXTERN int kill_grace_seconds; EXTERN int helper_ci; EXTERN int helper_pid; EXTERN int helper_kill_fd; diff --git a/src/timeouts.h b/src/timeouts.h index f62bb6f..80b9fc9 100644 --- a/src/timeouts.h +++ b/src/timeouts.h @@ -226,7 +226,7 @@ * * Working backward from the earlier watchdog firing at T170, leaving 10 seconds * for SIGKILL to succeed, we need to begin SIGKILL at T160. This means we - * have from T120 to T160 to allow graceful kill to complete. So, kill_count_grace + * have from T120 to T160 to allow graceful kill to complete. So, kill_grace_seconds * should be set to 40 by default (T120 to T160). * * T40: last successful disk renewal @@ -234,11 +234,6 @@ * T160 - T169: SIGKILL once per second (10 sec) * T170 - T179: watchdog fires sometime (SIGKILL continues) * T180: other hosts acquire our leases - * - * The interval between each kill count/attempt is approx 1 sec, - * so kill_count/kill_count_grace/kill_count_max serve as both - * the number/count of attempts and the number of seconds spent - * using that kind of termination. */