]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#1375] only recreate host managers that have at least one broken connection
authorRazvan Becheriu <razvan@isc.org>
Fri, 6 Nov 2020 17:18:31 +0000 (19:18 +0200)
committerRazvan Becheriu <razvan@isc.org>
Wed, 9 Dec 2020 17:12:46 +0000 (19:12 +0200)
src/lib/database/database_connection.h
src/lib/dhcpsrv/base_host_data_source.h
src/lib/dhcpsrv/host_data_source_factory.cc
src/lib/dhcpsrv/host_data_source_factory.h
src/lib/dhcpsrv/host_mgr.cc
src/lib/dhcpsrv/host_mgr.h
src/lib/dhcpsrv/mysql_host_data_source.cc
src/lib/dhcpsrv/mysql_host_data_source.h
src/lib/dhcpsrv/pgsql_host_data_source.cc
src/lib/dhcpsrv/pgsql_host_data_source.h
src/lib/mysql/mysql_connection.cc

index ba313a680a47815e765a189c1815f2e2e84d8e49..301f3a1378837554ef700dd9548c4bad9c85b0c9 100644 (file)
@@ -291,6 +291,13 @@ public:
         }
     }
 
+    /// @brief Flag which indicates if connection is unusable.
+    ///
+    /// @return true if the connection is unusable, false otherwise
+    bool isUnusable() {
+        return (unusable_);
+    }
+
 protected:
     /// @brief Sets the unusable flag to true.
     void markUnusable() { unusable_ = true; }
index c83a0bea34d12f59debc61fb026ba4522fa03224..522849739747c16df9459161e85efe368b2c7032 100644 (file)
@@ -499,6 +499,15 @@ public:
     /// @return true if the new setting was accepted by the backend or false
     /// otherwise.
     virtual bool setIPReservationsUnique(const bool unique) = 0;
+
+    /// @brief Flag which indicates if the host manager has at least one
+    /// unusable connection.
+    ///
+    /// @return true if there is at least one unusable connection, false
+    /// otherwise
+    virtual bool isUnusable() {
+        return (false);
+    }
 };
 
 /// @brief HostDataSource pointer
index 45817b614eaac3d48de61798fc92487343390500..bcbd03bfbb90ed9611afe37b6027c7c35999f0de 100644 (file)
@@ -103,7 +103,8 @@ HostDataSourceFactory::del(HostDataSourceList& sources,
 bool
 HostDataSourceFactory::del(HostDataSourceList& sources,
                            const string& db_type,
-                           const string& dbaccess) {
+                           const string& dbaccess,
+                           bool if_unusable) {
     DatabaseConnection::ParameterMap parameters =
             DatabaseConnection::parse(dbaccess);
     bool result = false;
@@ -111,6 +112,9 @@ HostDataSourceFactory::del(HostDataSourceList& sources,
         if ((*it)->getType() != db_type || (*it)->getParameters() != parameters) {
             continue;
         }
+        if (if_unusable && (!(*it)->isUnusable())) {
+            continue;
+        }
         LOG_DEBUG(hosts_logger, DHCPSRV_DBG_TRACE, HOSTS_CFG_CLOSE_HOST_DATA_SOURCE)
             .arg((*it)->getType());
         sources.erase(it);
index 97b9d2a6749ff13ad76575d4e54f9f35b32b64a6..aad11a907f3caa8c371d3730a8ad83ded056a549 100644 (file)
@@ -86,9 +86,11 @@ public:
     ///        "keyword=value" pairs, separated by spaces. They are backend-
     ///        -end specific, although must include the "type" keyword which
     ///        gives the backend in use.
+    /// @param if_unusable flag which indicates if the host data source should
+    ///        be deleted only if it is unusable.
     /// @return true when found and removed, false when not found.
     static bool del(HostDataSourceList& sources, const std::string& db_type,
-                    const std::string& dbaccess);
+                    const std::string& dbaccess, bool if_unusable = true);
 
     /// @brief Type of host data source factory
     ///
index 9a08018bf38b02b2396c013d53d95c3aae6aee1f..1397c48443302a138f31f04073989d6a2f897617 100644 (file)
@@ -55,12 +55,15 @@ HostMgr::delBackend(const std::string& db_type) {
         getHostMgrPtr()->cache_ptr_->getType() == db_type) {
         getHostMgrPtr()->cache_ptr_.reset();
     }
-    return (HostDataSourceFactory::del(getHostMgrPtr()->alternate_sources_, db_type));
+    return (HostDataSourceFactory::del(getHostMgrPtr()->alternate_sources_,
+                                       db_type));
 }
 
 bool
-HostMgr::delBackend(const std::string& db_type, const std::string& access) {
-    return (HostDataSourceFactory::del(getHostMgrPtr()->alternate_sources_, db_type, access));
+HostMgr::delBackend(const std::string& db_type, const std::string& access,
+                    bool if_unusable) {
+    return (HostDataSourceFactory::del(getHostMgrPtr()->alternate_sources_,
+                                       db_type, access, if_unusable));
 }
 
 void
index 0860761d8e3ac773af676e6b97e8336bea81ba30..21ee69612af78ec010555cd5564d053a0cf7deba 100644 (file)
@@ -85,8 +85,12 @@ public:
     /// @param db_type database backend type.
     /// @param access Host backend access parameters for the alternate
     /// host backend. It holds "keyword=value" pairs, separated by spaces.
+    /// @param if_unusable flag which indicates if the host data source should
+    /// be deleted only if it is unusable.
     /// @return true when found and removed, false when not found.
-    static bool delBackend(const std::string& db_type, const std::string& access);
+    static bool delBackend(const std::string& db_type,
+                           const std::string& access,
+                           bool if_unusable = false);
 
     /// @brief Delete all alternate backends.
     static void delAllBackends();
@@ -640,6 +644,7 @@ public:
     }
 
 protected:
+
     /// @brief The negative caching flag.
     ///
     /// When true and the first backend is a cache
index edb60eb8f9f6e118644bae6d97ffc7f4ec5565ed..a1dd6a3053ba5b131a3e10a9ae0bb1b101794c2a 100644 (file)
@@ -2258,6 +2258,10 @@ public:
 
     /// @brief The pool of contexts
     MySqlHostContextPoolPtr pool_;
+
+    /// @brief Indicates if there is at least one connection that can no longer
+    /// be used for normal operations.
+    bool unusable_;
 };
 
 namespace {
@@ -2727,7 +2731,7 @@ MySqlHostContext::MySqlHostContext(const DatabaseConnection::ParameterMap& param
 // MySqlHostContextAlloc Constructor and Destructor
 
 MySqlHostDataSource::MySqlHostContextAlloc::MySqlHostContextAlloc(
-    const MySqlHostDataSourceImpl& mgr) : ctx_(), mgr_(mgr) {
+    MySqlHostDataSourceImpl& mgr) : ctx_(), mgr_(mgr) {
 
     if (MultiThreadingMgr::instance().getMode()) {
         // multi-threaded
@@ -2756,12 +2760,16 @@ MySqlHostDataSource::MySqlHostContextAlloc::~MySqlHostContextAlloc() {
         // multi-threaded
         lock_guard<mutex> lock(mgr_.pool_->mutex_);
         mgr_.pool_->pool_.push_back(ctx_);
+        if (ctx_->conn_.isUnusable()) {
+            mgr_.unusable_ = true;
+        }
+    } else if (ctx_->conn_.isUnusable()) {
+        mgr_.unusable_ = true;
     }
-    // If running in single-threaded mode, there's nothing to do here.
 }
 
 MySqlHostDataSourceImpl::MySqlHostDataSourceImpl(const DatabaseConnection::ParameterMap& parameters)
-    : parameters_(parameters), ip_reservations_unique_(true) {
+    : parameters_(parameters), ip_reservations_unique_(true), unusable_(false) {
 
     // Validate the schema version first.
     std::pair<uint32_t, uint32_t> code_version(MYSQL_SCHEMA_VERSION_MAJOR,
@@ -2861,7 +2869,7 @@ MySqlHostDataSourceImpl::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
         std::list<std::string> host_db_access_list = cfg_db->getHostDbAccessStringList();
         for (std::string& hds : host_db_access_list) {
             auto parameters = DatabaseConnection::parse(hds);
-            if (HostMgr::delBackend("mysql", hds)) {
+            if (HostMgr::delBackend("mysql", hds, true)) {
                 HostMgr::addBackend(hds);
             }
         }
@@ -3927,6 +3935,10 @@ MySqlHostDataSource::setIPReservationsUnique(const bool unique) {
     return (true);
 }
 
+bool
+MySqlHostDataSource::isUnusable() {
+    return (impl_->unusable_);
+}
 
 }  // namespace dhcp
 }  // namespace isc
index be705d2081481b5af45641cc9e687f675e936e8d..879eb7e3b6b2a26e1d7f52aeb56b8de4d0cb2c48 100644 (file)
@@ -463,6 +463,13 @@ public:
     /// the addresses must be unique and when they may be non-unique.
     virtual bool setIPReservationsUnique(const bool unique);
 
+    /// @brief Flag which indicates if the host manager has at least one
+    /// unusable connection.
+    ///
+    /// @return true if there is at least one unusable connection, false
+    /// otherwise
+    virtual bool isUnusable();
+
     /// @brief Context RAII Allocator.
     class MySqlHostContextAlloc {
     public:
@@ -473,7 +480,7 @@ public:
         /// or creates a new one.
         ///
         /// @param mgr A parent instance
-        MySqlHostContextAlloc(const MySqlHostDataSourceImpl& mgr);
+        MySqlHostContextAlloc(MySqlHostDataSourceImpl& mgr);
 
         /// @brief Destructor
         ///
@@ -485,7 +492,7 @@ public:
 
     private:
         /// @brief The manager
-        const MySqlHostDataSourceImpl& mgr_;
+        MySqlHostDataSourceImpl& mgr_;
     };
 
 private:
index 7033b2fb07e7cadd4952d27fb8ef23dc97413ba4..2d941acbb808d2cff1435022c4fa0844487f5a50 100644 (file)
@@ -1610,6 +1610,10 @@ public:
 
     /// @brief The pool of contexts
     PgSqlHostContextPoolPtr pool_;
+
+    /// @brief Indicates if there is at least one connection that can no longer
+    /// be used for normal operations.
+    bool unusable_;
 };
 
 namespace {
@@ -2168,7 +2172,7 @@ PgSqlHostContext::PgSqlHostContext(const DatabaseConnection::ParameterMap& param
 // PgSqlHostContextAlloc Constructor and Destructor
 
 PgSqlHostDataSource::PgSqlHostContextAlloc::PgSqlHostContextAlloc(
-    const PgSqlHostDataSourceImpl& mgr) : ctx_(), mgr_(mgr) {
+    PgSqlHostDataSourceImpl& mgr) : ctx_(), mgr_(mgr) {
 
     if (MultiThreadingMgr::instance().getMode()) {
         // multi-threaded
@@ -2197,12 +2201,16 @@ PgSqlHostDataSource::PgSqlHostContextAlloc::~PgSqlHostContextAlloc() {
         // multi-threaded
         lock_guard<mutex> lock(mgr_.pool_->mutex_);
         mgr_.pool_->pool_.push_back(ctx_);
+        if (ctx_->conn_.isUnusable()) {
+            mgr_.unusable_ = true;
+        }
+    } else if (ctx_->conn_.isUnusable()) {
+        mgr_.unusable_ = true;
     }
-    // If running in single-threaded mode, there's nothing to do here.
 }
 
 PgSqlHostDataSourceImpl::PgSqlHostDataSourceImpl(const DatabaseConnection::ParameterMap& parameters)
-    : parameters_(parameters), ip_reservations_unique_(true) {
+    : parameters_(parameters), ip_reservations_unique_(true), unusable_(false) {
 
     // Validate the schema version first.
     std::pair<uint32_t, uint32_t> code_version(PG_SCHEMA_VERSION_MAJOR,
@@ -2297,7 +2305,7 @@ PgSqlHostDataSourceImpl::dbReconnect(ReconnectCtlPtr db_reconnect_ctl) {
         std::list<std::string> host_db_access_list = cfg_db->getHostDbAccessStringList();
         for (std::string& hds : host_db_access_list) {
             auto parameters = DatabaseConnection::parse(hds);
-            if (HostMgr::delBackend("postgresql", hds)) {
+            if (HostMgr::delBackend("postgresql", hds, true)) {
                 HostMgr::addBackend(hds);
             }
         }
@@ -3189,5 +3197,10 @@ PgSqlHostDataSource::setIPReservationsUnique(const bool unique) {
     return (true);
 }
 
+bool
+PgSqlHostDataSource::isUnusable() {
+    return (impl_->unusable_);
+}
+
 }  // namespace dhcp
 }  // namespace isc
index aec52305a35f28292288c87b1525947199992068..7fb251054a18d21a65ff61704bde0934c8491ce9 100644 (file)
@@ -514,6 +514,13 @@ public:
     /// the addresses must be unique and when they may be non-unique.
     virtual bool setIPReservationsUnique(const bool unique);
 
+    /// @brief Flag which indicates if the host manager has at least one
+    /// unusable connection.
+    ///
+    /// @return true if there is at least one unusable connection, false
+    /// otherwise
+    virtual bool isUnusable();
+
     /// @brief Context RAII Allocator.
     class PgSqlHostContextAlloc {
     public:
@@ -524,7 +531,7 @@ public:
         /// or creates a new one.
         ///
         /// @param mgr A parent instance
-        PgSqlHostContextAlloc(const PgSqlHostDataSourceImpl& mgr);
+        PgSqlHostContextAlloc(PgSqlHostDataSourceImpl& mgr);
 
         /// @brief Destructor
         ///
@@ -536,7 +543,7 @@ public:
 
     private:
         /// @brief The manager
-        const PgSqlHostDataSourceImpl& mgr_;
+        PgSqlHostDataSourceImpl& mgr_;
     };
 
 private:
index ce9a1086c87176926dba4924bc8ba552e21e7112..4d205ce14a84935b89bf01d7209ecf2a85646185 100644 (file)
@@ -52,7 +52,6 @@ MySqlTransaction::commit() {
 
 void
 MySqlConnection::openDatabase() {
-
     // Set up the values of the parameters
     const char* host = "localhost";
     string shost;
@@ -438,6 +437,5 @@ MySqlConnection::rollback() {
     }
 }
 
-
 } // namespace isc::db
 } // namespace isc