]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#1621] addressed comments
authorRazvan Becheriu <razvan@isc.org>
Wed, 17 Mar 2021 15:32:54 +0000 (17:32 +0200)
committerRazvan Becheriu <razvan@isc.org>
Mon, 29 Mar 2021 18:11:29 +0000 (21:11 +0300)
src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc
src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc
src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
src/lib/dhcpsrv/mysql_host_data_source.cc
src/lib/dhcpsrv/mysql_lease_mgr.cc
src/lib/dhcpsrv/pgsql_host_data_source.cc
src/lib/dhcpsrv/pgsql_lease_mgr.cc
src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.h
src/lib/mysql/mysql_connection.h
src/lib/pgsql/pgsql_connection.h

index b201ed711e750e7af719da4b919ce8e2c17d3ee9..309291da94b994a1dbad5c16e049cbf219f195ef 100644 (file)
@@ -3009,10 +3009,12 @@ MySqlConfigBackendDHCPv4Impl::MySqlConfigBackendDHCPv4Impl(const DatabaseConnect
                             tagged_statements.end());
 //                            tagged_statements.begin() + WRITE_STMTS_BEGIN);
 
+    // Create unique timer name per instance.
     timer_name_ = "MySqlConfigBackend4[";
     timer_name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     timer_name_ += "]DbReconnectTimer";
 
+    // Create ReconnectCtl for this connection.
     conn_.makeReconnectCtl(timer_name_);
 }
 
index 9425763e773613b551b04d0e955aec50aa3dd227..21036525f9042628db8bac345e21779f41e705d7 100644 (file)
@@ -3482,10 +3482,12 @@ MySqlConfigBackendDHCPv6Impl::MySqlConfigBackendDHCPv6Impl(const DatabaseConnect
                             tagged_statements.end());
 //                            tagged_statements.begin() + WRITE_STMTS_BEGIN);
 
+    // Create unique timer name per instance.
     timer_name_ = "MySqlConfigBackend6[";
     timer_name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     timer_name_ += "]DbReconnectTimer";
 
+    // Create ReconnectCtl for this connection.
     conn_.makeReconnectCtl(timer_name_);
 }
 
index 3a61b8564e1fd88d3600344d5071b1c06667166f..e4f18dbcab4c32065876e783c0bb4b2260111117 100644 (file)
@@ -4176,7 +4176,7 @@ public:
         isc::dhcp::CfgMgr::instance().clear();
     }
 
-    ~MySqlConfigBackendDHCPv4DbLostCallbackTest() {
+    virtual ~MySqlConfigBackendDHCPv4DbLostCallbackTest() {
         isc::db::DatabaseConnection::db_lost_callback_ = 0;
         isc::db::DatabaseConnection::db_recovered_callback_ = 0;
         isc::db::DatabaseConnection::db_failed_callback_ = 0;
@@ -4191,10 +4191,6 @@ public:
     /// appropriate schema and create a basic DB manager to
     /// wipe out any prior instance
     virtual void SetUp() {
-        isc::dhcp::MySqlConfigBackendImpl::setIOService(io_service_);
-        isc::db::DatabaseConnection::db_lost_callback_ = 0;
-        isc::db::DatabaseConnection::db_recovered_callback_ = 0;
-        isc::db::DatabaseConnection::db_failed_callback_ = 0;
         // Ensure we have the proper schema with no transient data.
         createMySQLSchema();
         isc::dhcp::CfgMgr::instance().clear();
@@ -4206,10 +4202,6 @@ public:
     /// Invoked by gtest upon test exit, we destroy the schema
     /// we created.
     virtual void TearDown() {
-        isc::dhcp::MySqlConfigBackendImpl::setIOService(isc::asiolink::IOServicePtr());
-        isc::db::DatabaseConnection::db_lost_callback_ = 0;
-        isc::db::DatabaseConnection::db_recovered_callback_ = 0;
-        isc::db::DatabaseConnection::db_failed_callback_ = 0;
         // If data wipe enabled, delete transient data otherwise destroy the schema
         destroyMySQLSchema();
         isc::dhcp::CfgMgr::instance().clear();
@@ -4239,10 +4231,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbRecoveredCallback was invoked
@@ -4251,10 +4243,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbFailedCallback was invoked
@@ -4263,10 +4255,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbRecoveredCallback was invoked after two reconnect
@@ -4276,10 +4268,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbFailedCallback was invoked after two reconnect
index b2df370343c7e3939b68402f9065199f14581a4a..f07bf48ee7ea73cdb58be68a03308ff52a49b44c 100644 (file)
@@ -4351,7 +4351,7 @@ public:
         isc::dhcp::CfgMgr::instance().clear();
     }
 
-    ~MySqlConfigBackendDHCPv6DbLostCallbackTest() {
+    virtual ~MySqlConfigBackendDHCPv6DbLostCallbackTest() {
         isc::db::DatabaseConnection::db_lost_callback_ = 0;
         isc::db::DatabaseConnection::db_recovered_callback_ = 0;
         isc::db::DatabaseConnection::db_failed_callback_ = 0;
@@ -4366,10 +4366,6 @@ public:
     /// appropriate schema and create a basic DB manager to
     /// wipe out any prior instance
     virtual void SetUp() {
-        isc::dhcp::MySqlConfigBackendImpl::setIOService(io_service_);
-        isc::db::DatabaseConnection::db_lost_callback_ = 0;
-        isc::db::DatabaseConnection::db_recovered_callback_ = 0;
-        isc::db::DatabaseConnection::db_failed_callback_ = 0;
         // Ensure we have the proper schema with no transient data.
         createMySQLSchema();
         isc::dhcp::CfgMgr::instance().clear();
@@ -4381,10 +4377,6 @@ public:
     /// Invoked by gtest upon test exit, we destroy the schema
     /// we created.
     virtual void TearDown() {
-        isc::dhcp::MySqlConfigBackendImpl::setIOService(isc::asiolink::IOServicePtr());
-        isc::db::DatabaseConnection::db_lost_callback_ = 0;
-        isc::db::DatabaseConnection::db_recovered_callback_ = 0;
-        isc::db::DatabaseConnection::db_failed_callback_ = 0;
         // If data wipe enabled, delete transient data otherwise destroy the schema
         destroyMySQLSchema();
         isc::dhcp::CfgMgr::instance().clear();
@@ -4414,10 +4406,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbRecoveredCallback was invoked
@@ -4426,10 +4418,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbFailedCallback was invoked
@@ -4438,10 +4430,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbRecoveredCallback was invoked after two reconnect
@@ -4451,10 +4443,10 @@ public:
     /// @brief Verifies the CB manager's behavior if DB connection is lost
     ///
     /// This function creates a CB manager with a back end that supports
-    /// connectivity lost callback (currently only MySQL and PostgreSQL). It
-    /// verifies connectivity by issuing a known valid query. Next it simulates
-    /// connectivity lost by identifying and closing the socket connection to
-    /// the CB backend. It then reissues the query and verifies that:
+    /// connectivity lost callback. It verifies connectivity by issuing a known
+    /// valid query. Next it simulates connectivity lost by identifying and
+    /// closing the socket connection to the CB backend. It then reissues the
+    /// query and verifies that:
     /// -# The Query throws  DbOperationError (rather than exiting)
     /// -# The registered DbLostCallback was invoked
     /// -# The registered DbFailedCallback was invoked after two reconnect
index f4818dcdf219cd0eadcae6dde46cf668b11b9a19..e2e482213d741cdd5a301811198009d32c7c753e 100644 (file)
@@ -2781,6 +2781,7 @@ MySqlHostDataSourceImpl::MySqlHostDataSourceImpl(const DatabaseConnection::Param
     : parameters_(parameters), ip_reservations_unique_(true), unusable_(false),
       timer_name_("") {
 
+    // Create unique timer name per instance.
     timer_name_ = "MySqlHostMgr[";
     timer_name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     timer_name_ += "]DbReconnectTimer";
@@ -2841,6 +2842,7 @@ MySqlHostDataSourceImpl::createContext() const {
     ctx->host_ipv6_reservation_exchange_.reset(new MySqlIPv6ReservationExchange());
     ctx->host_option_exchange_.reset(new MySqlOptionExchange());
 
+    // Create ReconnectCtl for this connection.
     ctx->conn_.makeReconnectCtl(timer_name_);
 
     return (ctx);
@@ -2872,7 +2874,6 @@ MySqlHostDataSourceImpl::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
                 HostMgr::addBackend(hds);
             }
         }
-
         reopened = true;
     } catch (const std::exception& ex) {
         LOG_ERROR(dhcpsrv_logger, DHCPSRV_MYSQL_HOST_DB_RECONNECT_ATTEMPT_FAILED)
@@ -2902,7 +2903,6 @@ MySqlHostDataSourceImpl::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
 
             // Invoke application layer connection failed callback.
             DatabaseConnection::invokeDbFailedCallback(db_reconnect_ctl);
-
             return (false);
         }
 
index f1065c0797f7a83db49a411f687cb8ccb5ea1a8f..2c9abe76a5c444fad3574d2894df49087624ec5b 100644 (file)
@@ -1783,6 +1783,7 @@ MySqlLeaseMgr::MySqlLeaseContextAlloc::~MySqlLeaseContextAlloc() {
 MySqlLeaseMgr::MySqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters)
     : parameters_(parameters), timer_name_("") {
 
+    // Create unique timer name per instance.
     timer_name_ = "MySqlLeaseMgr[";
     timer_name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     timer_name_ += "]DbReconnectTimer";
@@ -1825,7 +1826,6 @@ MySqlLeaseMgr::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
         CfgDbAccessPtr cfg_db = CfgMgr::instance().getCurrentCfg()->getCfgDbAccess();
         LeaseMgrFactory::destroy();
         LeaseMgrFactory::create(cfg_db->getLeaseDbAccessString());
-
         reopened = true;
     } catch (const std::exception& ex) {
         LOG_ERROR(dhcpsrv_logger, DHCPSRV_MYSQL_LEASE_DB_RECONNECT_ATTEMPT_FAILED)
@@ -1855,7 +1855,6 @@ MySqlLeaseMgr::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
 
             // Invoke application layer connection failed callback.
             DatabaseConnection::invokeDbFailedCallback(db_reconnect_ctl);
-
             return (false);
         }
 
@@ -1897,6 +1896,7 @@ MySqlLeaseMgr::createContext() const {
     ctx->exchange4_.reset(new MySqlLease4Exchange());
     ctx->exchange6_.reset(new MySqlLease6Exchange());
 
+    // Create ReconnectCtl for this connection.
     ctx->conn_.makeReconnectCtl(timer_name_);
 
     return (ctx);
index d4dba22f0baf840f0ceb35f9c0b2775388baa5d1..6fec6459a97d597775a73879e90bd27398193f79 100644 (file)
@@ -2222,6 +2222,7 @@ PgSqlHostDataSourceImpl::PgSqlHostDataSourceImpl(const DatabaseConnection::Param
     : parameters_(parameters), ip_reservations_unique_(true), unusable_(false),
       timer_name_("") {
 
+    // Create unique timer name per instance.
     timer_name_ = "PgSqlHostMgr[";
     timer_name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     timer_name_ += "]DbReconnectTimer";
@@ -2277,6 +2278,7 @@ PgSqlHostDataSourceImpl::createContext() const {
     ctx->host_ipv6_reservation_exchange_.reset(new PgSqlIPv6ReservationExchange());
     ctx->host_option_exchange_.reset(new PgSqlOptionExchange());
 
+    // Create ReconnectCtl for this connection.
     ctx->conn_.makeReconnectCtl(timer_name_);
 
     return (ctx);
@@ -2308,7 +2310,6 @@ PgSqlHostDataSourceImpl::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
                 HostMgr::addBackend(hds);
             }
         }
-
         reopened = true;
     } catch (const std::exception& ex) {
         LOG_ERROR(dhcpsrv_logger, DHCPSRV_PGSQL_HOST_DB_RECONNECT_ATTEMPT_FAILED)
@@ -2338,7 +2339,6 @@ PgSqlHostDataSourceImpl::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
 
             // Invoke application layer connection failed callback.
             DatabaseConnection::invokeDbFailedCallback(db_reconnect_ctl);
-
             return (false);
         }
 
index da39f2d973182ae52ddeda8734456b234c227475..eed79d2eecf13db3baa4a642af6f2a0eedd4388b 100644 (file)
@@ -1217,6 +1217,7 @@ PgSqlLeaseMgr::PgSqlLeaseContextAlloc::~PgSqlLeaseContextAlloc() {
 PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters)
     : parameters_(parameters), timer_name_("") {
 
+    // Create unique timer name per instance.
     timer_name_ = "PgSqlLeaseMgr[";
     timer_name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     timer_name_ += "]DbReconnectTimer";
@@ -1259,7 +1260,6 @@ PgSqlLeaseMgr::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
         CfgDbAccessPtr cfg_db = CfgMgr::instance().getCurrentCfg()->getCfgDbAccess();
         LeaseMgrFactory::destroy();
         LeaseMgrFactory::create(cfg_db->getLeaseDbAccessString());
-
         reopened = true;
     } catch (const std::exception& ex) {
         LOG_ERROR(dhcpsrv_logger, DHCPSRV_PGSQL_LEASE_DB_RECONNECT_ATTEMPT_FAILED)
@@ -1289,7 +1289,6 @@ PgSqlLeaseMgr::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
 
             // Invoke application layer connection failed callback.
             DatabaseConnection::invokeDbFailedCallback(db_reconnect_ctl);
-
             return (false);
         }
 
@@ -1339,6 +1338,7 @@ PgSqlLeaseMgr::createContext() const {
     ctx->exchange4_.reset(new PgSqlLease4Exchange());
     ctx->exchange6_.reset(new PgSqlLease6Exchange());
 
+    // Create ReconnectCtl for this connection.
     ctx->conn_.makeReconnectCtl(timer_name_);
 
     return (ctx);
index 18baa31ec67a9109f3c9381ec607e6f37862b1c6..8444f9370476c089c27c1980e494c01316059ed4 100644 (file)
@@ -549,7 +549,7 @@ public:
         isc::dhcp::CfgMgr::instance().clear();
     }
 
-    ~HostMgrDbLostCallbackTest() {
+    virtual ~HostMgrDbLostCallbackTest() {
         isc::db::DatabaseConnection::db_lost_callback_ = 0;
         isc::db::DatabaseConnection::db_recovered_callback_ = 0;
         isc::db::DatabaseConnection::db_failed_callback_ = 0;
@@ -564,10 +564,6 @@ public:
     /// appropriate schema and create a basic host manager to
     /// wipe out any prior instance
     virtual void SetUp() {
-        isc::dhcp::HostMgr::setIOService(io_service_);
-        isc::db::DatabaseConnection::db_lost_callback_ = 0;
-        isc::db::DatabaseConnection::db_recovered_callback_ = 0;
-        isc::db::DatabaseConnection::db_failed_callback_ = 0;
         // Ensure we have the proper schema with no transient data.
         createSchema();
         // Wipe out any pre-existing mgr
@@ -580,10 +576,6 @@ public:
     /// Invoked by gtest upon test exit, we destroy the schema
     /// we created.
     virtual void TearDown() {
-        isc::dhcp::HostMgr::setIOService(isc::asiolink::IOServicePtr());
-        isc::db::DatabaseConnection::db_lost_callback_ = 0;
-        isc::db::DatabaseConnection::db_recovered_callback_ = 0;
-        isc::db::DatabaseConnection::db_failed_callback_ = 0;
         // If data wipe enabled, delete transient data otherwise destroy the schema
         destroySchema();
         isc::dhcp::CfgMgr::instance().clear();
index 8700289818e79441733fa7ddbf1de72e078cb530..43e804eb5704a2bf62a29fca622ce0a04ba13ee2 100644 (file)
@@ -658,11 +658,15 @@ public:
     ///
     /// @note The recover function must be run on the IO Service thread.
     void startRecoverDbConnection() {
-        if (!io_service_ && io_service_access_callback_) {
-            io_service_ = (*io_service_access_callback_)();
-        }
-        if (callback_ && io_service_) {
-            io_service_->post(std::bind(callback_, reconnectCtl()));
+        if (callback_) {
+            if (!io_service_ && io_service_access_callback_) {
+                io_service_ = (*io_service_access_callback_)();
+                io_service_access_callback_.reset();
+            }
+
+            if (io_service_) {
+                io_service_->post(std::bind(callback_, reconnectCtl()));
+            }
         }
     }
 
index da69a7a99bdedf618b5c3bfa83777c9af38798af..c2503b77ea59bf35e17f6272e18021a26692a67e 100644 (file)
@@ -426,11 +426,15 @@ public:
     ///
     /// @note The recover function must be run on the IO Service thread.
     void startRecoverDbConnection() {
-        if (!io_service_ && io_service_access_callback_) {
-            io_service_ = (*io_service_access_callback_)();
-        }
-        if (callback_ && io_service_) {
-            io_service_->post(std::bind(callback_, reconnectCtl()));
+        if (callback_) {
+            if (!io_service_ && io_service_access_callback_) {
+                io_service_ = (*io_service_access_callback_)();
+                io_service_access_callback_.reset();
+            }
+
+            if (io_service_) {
+                io_service_->post(std::bind(callback_, reconnectCtl()));
+            }
         }
     }