From: Francis Dupont <fdupont@isc.org>
Date: Wed, 13 Nov 2019 14:09:02 +0000 (+0100)
Subject: [960-mysql-connection-pool] Moved schema version checking
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c10492332fa3f62b3ca2e752778fb22f706392c9;p=thirdparty%2Fkea.git

[960-mysql-connection-pool] Moved schema version checking
---

diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index c4edec0607..dcaf487203 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -1725,8 +1725,21 @@ MySqlLeaseMgr::MySqlLeaseContextAlloc::~MySqlLeaseContextAlloc() {
 MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters)
     : parameters_(parameters) {
 
+    // Test schema version.
+    std::pair<uint32_t, uint32_t> code_version(MYSQL_SCHEMA_VERSION_MAJOR,
+                                               MYSQL_SCHEMA_VERSION_MINOR);
+    std::pair<uint32_t, uint32_t> db_version = getVersion();
+    if (code_version != db_version) {
+        isc_throw(DbOpenError,
+                  "MySQL schema version mismatch: need version: "
+                      << code_version.first << "." << code_version.second
+                      << " found version:  " << db_version.first << "."
+                      << db_version.second);
+    }
+
+    // Create an initial context.
     pool_.reset(new MySqlLeaseContextPool());
-    pool_->pool_.push_back(createContext(true));
+    pool_->pool_.push_back(createContext());
 }
 
 MySqlLeaseMgr::~MySqlLeaseMgr() {
@@ -1737,27 +1750,13 @@ MySqlLeaseMgr::~MySqlLeaseMgr() {
 // Create context.
 
 MySqlLeaseContextPtr
-MySqlLeaseMgr::createContext(bool check_version /* = false */) const {
+MySqlLeaseMgr::createContext() const {
 
     MySqlLeaseContextPtr ctx(new MySqlLeaseContext(parameters_));
 
     // Open the database.
     ctx->conn_.openDatabase();
 
-    // Test schema version before we try to prepare statements.
-    if (check_version) {
-        std::pair<uint32_t, uint32_t> code_version(MYSQL_SCHEMA_VERSION_MAJOR,
-                                                   MYSQL_SCHEMA_VERSION_MINOR);
-        std::pair<uint32_t, uint32_t> db_version = getVersion(ctx->conn_);
-        if (code_version != db_version) {
-            isc_throw(DbOpenError,
-                      "MySQL schema version mismatch: need version: "
-                      << code_version.first << "." << code_version.second
-                      << " found version:  " << db_version.first << "."
-                      << db_version.second);
-        }
-    }
-
     // Enable autocommit.  To avoid a flush to disk on every commit, the global
     // parameter innodb_flush_log_at_trx_commit should be set to 2.  This will
     // cause the changes to be written to the log, but flushed to disk in the
@@ -2959,21 +2958,15 @@ MySqlLeaseMgr::getDescription() const {
 
 std::pair<uint32_t, uint32_t>
 MySqlLeaseMgr::getVersion() const {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MYSQL_GET_VERSION);
+
     // Get a connection.
     MySqlConnection conn(parameters_);
 
     // Open the database.
     conn.openDatabase();
 
-    // Get the version.
-    return (getVersion(conn));
-}
-
-std::pair<uint32_t, uint32_t>
-MySqlLeaseMgr::getVersion(MySqlConnection& conn) const {
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MYSQL_GET_VERSION);
-
     // Allocate a new statement.
     MYSQL_STMT *stmt = mysql_stmt_init(conn.mysql_);
     if (stmt == NULL) {
diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h
index c89a12c7a9..2d710179a7 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.h
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.h
@@ -87,7 +87,7 @@ public:
     /// - user - Username under which to connect (optional)
     /// - password - Password for "user" on the database (optional)
     ///
-    /// Create an initial context checking the schema version.
+    /// Check the schema version and create an initial context.
     ///
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
@@ -104,20 +104,13 @@ public:
 
     /// #brief Create a new context.
     ///
-    /// The database in opened.
-    /// If wanted the version number in the schema_version table will be
-    /// checked against hard-coded value in the implementation file.
-    /// Finally, all the SQL commands are pre-compiled.
+    /// The database in opened withh all the SQL commands pre-compiled.
     ///
-    /// @param check_version If true and the database successfully opened
-    /// check the schema version.
     /// @return A new (never null) context.
     /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given.
-    /// @throw isc::db::DbOpenError Error opening the database or the schema
-    /// version is incorrect.
     /// @throw isc::db::DbOperationError An operation on the open database has
     /// failed.
-    MySqlLeaseContextPtr createContext(bool check_version = false) const;
+    MySqlLeaseContextPtr createContext() const;
 
     /// @brief Local version of getDBVersion() class method
     static std::string getDBVersion();
@@ -671,21 +664,6 @@ public:
     };
 
 private:
-    /// @brief Returns backend version.
-    ///
-    /// The method is called by the constructor after opening the database
-    /// but prior to preparing SQL statements, to verify that the schema version
-    /// is correct. Thus it must not rely on a pre-prepared statement or
-    /// formal statement execution error checking.
-    ///
-    /// @param conn Initial connecion.
-    /// @return Version number as a pair of unsigned integers.  "first" is the
-    ///         major version number, "second" the minor number.
-    ///
-    /// @throw isc::db::DbOperationError An operation on the open database has
-    ///        failed.
-    virtual std::pair<uint32_t, uint32_t> getVersion(db::MySqlConnection& conn) const;
-
     /// @brief Add Lease Common Code
     ///
     /// This method performs the common actions for both flavours (V4 and V6)