From ae1272d37057e22416efe5cb48e23f06bd9e30ff Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 31 Jan 2020 12:02:57 -0500 Subject: [PATCH] [#1108] Servers execute shutdown on unrecoverable DBs Added ChangeLog entry src/bin/dhcp4/ctrl_dhcp4_srv.* ControlledDhcpv4Srv::dbLostCallback() - schedules a shutdown once retries have been exhausted/disableld src/bin/dhcp6/ctrl_dhcp6_srv.* ControlledDhcpv6Srv::dbLostCallback() - schedules a shutdown once retries have been exhausted/disableld src/lib/database/database_connection.h class DbUnrecoverableError - new exception src/lib/mysql/mysql_connection.h MySqlConnection::check_error() - throws DbUnrecoverableError instead of calling exit() src/lib/pgsql/pgsql_connection.* PgSqlConnection::checkStatementError() - throws DbUnrecoverableError instead of calling exit() --- ChangeLog | 9 +++++++++ src/bin/dhcp4/ctrl_dhcp4_srv.cc | 6 ++++-- src/bin/dhcp4/ctrl_dhcp4_srv.h | 4 ++-- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 1 + src/bin/dhcp6/ctrl_dhcp6_srv.h | 4 ++-- src/lib/database/database_connection.h | 10 +++++++++- src/lib/mysql/mysql_connection.h | 9 +++++---- src/lib/pgsql/pgsql_connection.cc | 5 +++-- src/lib/pgsql/pgsql_connection.h | 4 ++-- 9 files changed, 37 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4c67b3b851..c4b9b0527e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +1718. [bug] tmark + kea-dhcp4 and kea-dhcp6 now shutdown gracefully by executing + the shutdown command, if connectivity with a backend database + has been lost and retries are either disabled or have been + exhausted. Prior to this they simply invoked exit() which + could orphan control socket files or cause segfaults unloading + the CB Cmds hook library. + (Gitlab #1108) + 1717. [func] razvan Prepared PgSqlHostMgr to be used with multi-threading by using a connection pool with thread context. diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index f40a38652e..c5f4931330 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2020 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -1086,12 +1086,14 @@ ControlledDhcpv4Srv::dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) { return (false); } - // If reconnect isn't enabled, log it and return false + // If reconnect isn't enabled or we're out of retries, + // log it, schedule a shutdown, and return false if (!db_reconnect_ctl->retriesLeft() || !db_reconnect_ctl->retryInterval()) { LOG_INFO(dhcp4_logger, DHCP4_DB_RECONNECT_DISABLED) .arg(db_reconnect_ctl->retriesLeft()) .arg(db_reconnect_ctl->retryInterval()); + ControlledDhcpv4Srv::processCommand("shutdown", ConstElementPtr()); return(false); } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 503acffb5f..0111c9f7b7 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -387,8 +387,8 @@ private: /// between retry attempts. /// /// If either value is zero, reconnect is presumed to be disabled and - /// the function will returns false. This instructs the DB backend - /// layer (the caller) to treat the connectivity loss as fatal. + /// the function will schedule a shutdown and return false. This instructs + /// the DB backend layer (the caller) to treat the connectivity loss as fatal. /// /// Otherwise, the function saves db_reconnect_ctl and invokes /// dbReconnect to initiate the reconnect process. diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 7445f8a41e..0aab657707 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -1110,6 +1110,7 @@ ControlledDhcpv6Srv::dbLostCallback(ReconnectCtlPtr db_reconnect_ctl) { LOG_INFO(dhcp6_logger, DHCP6_DB_RECONNECT_DISABLED) .arg(db_reconnect_ctl->retriesLeft()) .arg(db_reconnect_ctl->retryInterval()); + ControlledDhcpv6Srv::processCommand("shutdown", ConstElementPtr()); return(false); } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 12983d1875..f4085fab90 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -386,8 +386,8 @@ private: /// between retry attempts. /// /// If either value is zero, reconnect is presumed to be disabled and - /// the function will returns false. This instructs the DB backend - /// layer (the caller) to treat the connectivity loss as fatal. + /// the function will schedule a shutdown and return false. This instructs + /// the DB backend layer (the caller) to treat the connectivity loss as fatal. /// /// Otherwise, the function saves db_reconnect_ctl and invokes /// dbReconnect to initiate the reconnect process. diff --git a/src/lib/database/database_connection.h b/src/lib/database/database_connection.h index 22336b66bd..a4164f3341 100644 --- a/src/lib/database/database_connection.h +++ b/src/lib/database/database_connection.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2020 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -39,6 +39,14 @@ public: isc::Exception(file, line, what) {} }; +/// @brief Exception thrown when connectivity has been lost and +/// cannot be recovered. +class DbUnrecoverableError : public Exception { +public: + DbUnrecoverableError(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) {} +}; + /// @brief Invalid type exception /// /// Thrown when the factory doesn't recognize the type of the backend. diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h index 6b18f65fbc..1ec795325b 100644 --- a/src/lib/mysql/mysql_connection.h +++ b/src/lib/mysql/mysql_connection.h @@ -543,8 +543,8 @@ public: /// If the error is deemed unrecoverable, such as a loss of connectivity /// with the server, the function will call invokeDbLostCallback(). If the /// invocation returns false then either there is no callback registered - /// or the callback has elected not to attempt to reconnect, and exit(-1) - /// is called; + /// or the callback has elected not to attempt to reconnect, and a + /// DbUnrecoverableError is thrown. /// /// If the invocation returns true, this indicates the calling layer will /// attempt recovery, and the function throws a DbOperationError to allow @@ -581,9 +581,10 @@ public: .arg(mysql_errno(mysql_)); // If there's no lost db callback or it returns false, - // then we're not attempting to recover so we're done + // then we're not attempting to recover so we're done. if (!invokeDbLostCallback()) { - exit (-1); + isc_throw(db::DbUnrecoverableError, + "database connectivity cannot be recovered"); } // We still need to throw so caller can error out of the current diff --git a/src/lib/pgsql/pgsql_connection.cc b/src/lib/pgsql/pgsql_connection.cc index 8e07e661f4..0f22511db1 100644 --- a/src/lib/pgsql/pgsql_connection.cc +++ b/src/lib/pgsql/pgsql_connection.cc @@ -336,9 +336,10 @@ PgSqlConnection::checkStatementError(const PgSqlResult& r, .arg(sqlstate ? sqlstate : ""); // If there's no lost db callback or it returns false, - // then we're not attempting to recover so we're done + // then we're not attempting to recover so we're done. if (!invokeDbLostCallback()) { - exit (-1); + isc_throw(db::DbUnrecoverableError, + "database connectivity cannot be recovered"); } // We still need to throw so caller can error out of the current diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h index ac644a9e75..d9ffe1e291 100644 --- a/src/lib/pgsql/pgsql_connection.h +++ b/src/lib/pgsql/pgsql_connection.h @@ -399,8 +399,8 @@ public: /// If the error is deemed unrecoverable, such as a loss of connectivity /// with the server, the function will call invokeDbLostCallback(). If the /// invocation returns false then either there is no callback registered - /// or the callback has elected not to attempt to reconnect, and exit(-1) - /// is called; + /// or the callback has elected not to attempt to reconnect, and a + /// DbUnrecoverableError is thrown. /// /// If the invocation returns true, this indicates the calling layer will /// attempt recovery, and the function throws a DbOperationError to allow