[5629] MySQL Lease back end now validates schema after connecting

src/lib/dhcpsrv/mysql_lease_mgr.*
    MySqlLeaseMgr::MySqlLeaseMgr() - now validates schema
    after connecting

    MySqlLeaseMgr::getVersion()  - no longer relies on
    pre-prepared statement or formal statement execution
    error handling

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
    LeaseMgrDbLostCallbackTest::testDbLostCallback() - replaced
    use of getVersion() with getLease4() for testing DB usability

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -209,8 +209,6 @@ tagged_statements = { {
                             "WHERE state != ? AND expire < ? "
                             "ORDER BY expire ASC "
                             "LIMIT ?"},
-    {MySqlLeaseMgr::GET_VERSION,
-                    "SELECT version, minor FROM schema_version"},
     {MySqlLeaseMgr::INSERT_LEASE4,
                     "INSERT INTO lease4(address, hwaddr, client_id, "
                         "valid_lifetime, expire, subnet_id, "
@@ -1501,6 +1499,16 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters)
     // Open the database.
     conn_.openDatabase();
 
+    // Test schema version before we try to prepare statements.
+    std::pair<uint32_t, uint32_t> version = getVersion();
+    if (version.first != MYSQL_SCHEMA_VERSION_MAJOR ||
+        version.second != MYSQL_SCHEMA_VERSION_MINOR) {
+        isc_throw(DbOpenError, "MySQL schema version is: "
+                  << version.first << "." << version.second << ", need version: "
+                  << MYSQL_SCHEMA_VERSION_MAJOR << "."
+                  << MYSQL_SCHEMA_VERSION_MINOR);
+    }
+
     // 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
@@ -2372,40 +2380,60 @@ MySqlLeaseMgr::getDescription() const {
 
 std::pair<uint32_t, uint32_t>
 MySqlLeaseMgr::getVersion() const {
-    const StatementIndex stindex = GET_VERSION;
-
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
               DHCPSRV_MYSQL_GET_VERSION);
 
-    uint32_t    major;      // Major version number
+    // Allocate a new statement.
-    uint32_t    minor;      // Minor version number
+    MYSQL_STMT *stmt = mysql_stmt_init(conn_.mysql_);
+    if (stmt == NULL) {
+        isc_throw(DbOperationError, "unable to allocate MySQL prepared "
+                "statement structure, reason: " << mysql_error(conn_.mysql_));
+    }
 
-    // Execute the prepared statement
+    // Prepare the statement from SQL text.
-    int status = mysql_stmt_execute(conn_.statements_[stindex]);
+    const char* version_sql = "SELECT version, minor FROM schema_version";
-    checkError(status, stindex, "unable to execute statement");
+    int status = mysql_stmt_prepare(stmt, version_sql, strlen(version_sql));
+    if (status != 0) {
+        isc_throw(DbOperationError, "unable to prepare MySQL statement <"
+                  << version_sql << ">, reason: " << mysql_error(conn_.mysql_));
+    }
+
+    // Execute the prepared statement.
+    if (mysql_stmt_execute(stmt) != 0) {
+        isc_throw(DbOperationError, "cannot execute schema version query:"
+                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")");
+    }
 
     // Bind the output of the statement to the appropriate variables.
     MYSQL_BIND bind[2];
     memset(bind, 0, sizeof(bind));
 
+    uint32_t major;
     bind[0].buffer_type = MYSQL_TYPE_LONG;
     bind[0].is_unsigned = 1;
     bind[0].buffer = &major;
     bind[0].buffer_length = sizeof(major);
 
+    uint32_t minor;
     bind[1].buffer_type = MYSQL_TYPE_LONG;
     bind[1].is_unsigned = 1;
     bind[1].buffer = &minor;
     bind[1].buffer_length = sizeof(minor);
 
-    status = mysql_stmt_bind_result(conn_.statements_[stindex], bind);
+    if (mysql_stmt_bind_result(stmt, bind)) {
-    checkError(status, stindex, "unable to bind result set");
+        isc_throw(DbOperationError, "unable to bind result set for:"
+                << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")");
+    }
 
-    // Fetch the data and set up the "release" object to release associated
+    // Fetch the data.
-    // resources when this method exits then retrieve the data.
+    if (mysql_stmt_fetch(stmt)) {
-    MySqlFreeResult fetch_release(conn_.statements_[stindex]);
+        mysql_stmt_close(stmt);
-    status = mysql_stmt_fetch(conn_.statements_[stindex]);
+        isc_throw(DbOperationError, "unable to bind result set for:"
-    checkError(status, stindex, "unable to fetch result set");
+                << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")");
+    }
+
+    // Discard the statement and its resources
+    mysql_stmt_close(stmt);
 
     return (std::make_pair(major, minor));
 }

src/lib/dhcpsrv/mysql_lease_mgr.h

@@ -53,7 +53,8 @@ public:
     ///        concerned with the database.
     ///
     /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given
-    /// @throw isc::dhcp::DbOpenError Error opening the database
+    /// @throw isc::dhcp::DbOpenError Error opening the database or the schema
+    /// version is incorrect.
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
     MySqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters);
@@ -472,6 +473,11 @@ public:
 
     /// @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 does not rely on a pre-prepared statement or
+    /// formal statement execution error checking.
+    ///
     /// @return Version number as a pair of unsigned integers.  "first" is the
     ///         major version number, "second" the minor number.
     ///
@@ -517,7 +523,6 @@ public:
         GET_LEASE6_DUID_IAID_SUBID,  // Get lease6 by DUID, IAID and subnet ID
         GET_LEASE6_SUBID,            // Get IPv6 leases by subnet ID
         GET_LEASE6_EXPIRE,           // Get lease6 by expiration.
-        GET_VERSION,                 // Obtain version number
         INSERT_LEASE4,               // Add entry to lease4 table
         INSERT_LEASE6,               // Add entry to lease6 table
         UPDATE_LEASE4,               // Update a Lease4 entry

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -2842,16 +2842,19 @@ LeaseMgrDbLostCallbackTest::testDbLostCallback() {
     // Clear the callback invocation marker.
     callback_called_ = false;
 
-    // Verify we can execute a query.
+    // Verify we can execute a query.  We do not care if
+    // we find a lease or not.
     LeaseMgr& lm = LeaseMgrFactory::instance();
-    pair<uint32_t, uint32_t> version;
+
-    ASSERT_NO_THROW(version = lm.getVersion());
+    Lease4Ptr lease;
+    ASSERT_NO_THROW(lease = lm.getLease4(IOAddress("192.0.1.0")));
 
     // Now close the sql socket out from under backend client
     ASSERT_EQ(0, close(sql_socket));
 
     // A query should fail with DbOperationError.
-    ASSERT_THROW(version = lm.getVersion(), DbOperationError);
+    ASSERT_THROW(lease = lm.getLease4(IOAddress("192.0.1.0")),
+                 DbOperationError);
 
     // Our lost connectivity callback should have been invoked.
     EXPECT_TRUE(callback_called_);