Browse Source

[4489] Added support for read only mode in MySQL HR backend.

Marcin Siodelski 8 years ago
parent
commit
65d3817db8

+ 11 - 6
src/lib/dhcpsrv/mysql_connection.cc

@@ -213,22 +213,27 @@ MySqlConnection::prepareStatement(uint32_t index, const char* text) {
 }
 
 void
-MySqlConnection::prepareStatements(const TaggedStatement tagged_statements[],
+MySqlConnection::prepareStatements(const TaggedStatement* start_statement,
+                                   const TaggedStatement* end_statement,
                                    size_t num_statements) {
     // Allocate space for all statements
-    statements_.clear();
     statements_.resize(num_statements, NULL);
 
-    text_statements_.clear();
     text_statements_.resize(num_statements, std::string(""));
 
     // Created the MySQL prepared statements for each DML statement.
-    for (int i = 0; tagged_statements[i].text != NULL; ++i) {
-        prepareStatement(tagged_statements[i].index,
-                         tagged_statements[i].text);
+    for (const TaggedStatement* tagged_statement = start_statement;
+         tagged_statement != end_statement; ++tagged_statement) {
+        prepareStatement(tagged_statement->index,
+                         tagged_statement->text);
     }
 }
 
+void MySqlConnection::clearStatements() {
+    statements_.clear();
+    text_statements_.clear();
+}
+
 /// @brief Destructor
 MySqlConnection::~MySqlConnection() {
     // Free up the prepared statements, ignoring errors. (What would we do

+ 5 - 1
src/lib/dhcpsrv/mysql_connection.h

@@ -247,9 +247,13 @@ public:
     ///        failed.
     /// @throw isc::InvalidParameter 'index' is not valid for the vector.  This
     ///        represents an internal error within the code.
-    void prepareStatements(const TaggedStatement tagged_statements[],
+    void prepareStatements(const TaggedStatement* start_statement,
+                           const TaggedStatement* end_statement,
                            size_t num_statements);
 
+    /// @brief Clears prepared statements and text statements.
+    void clearStatements();
+
     /// @brief Open Database
     ///
     /// Opens the database using the information supplied in the parameters

+ 94 - 45
src/lib/dhcpsrv/mysql_host_data_source.cc

@@ -19,6 +19,7 @@
 
 #include <boost/algorithm/string/split.hpp>
 #include <boost/algorithm/string/classification.hpp>
+#include <boost/array.hpp>
 #include <boost/pointer_cast.hpp>
 #include <boost/static_assert.hpp>
 
@@ -1633,10 +1634,6 @@ public:
     ///
     /// The contents of the enum are indexes into the list of SQL statements
     enum StatementIndex {
-        INSERT_HOST,            // Insert new host to collection
-        INSERT_V6_RESRV,        // Insert v6 reservation
-        INSERT_V4_OPTION,       // Insert DHCPv4 option
-        INSERT_V6_OPTION,       // Insert DHCPv6 option
         GET_HOST_DHCPID,        // Gets hosts by host identifier
         GET_HOST_ADDR,          // Gets hosts by IPv4 address
         GET_HOST_SUBID4_DHCPID, // Gets host by IPv4 SubnetID, HW address/DUID
@@ -1644,9 +1641,20 @@ public:
         GET_HOST_SUBID_ADDR,    // Gets host by IPv4 SubnetID and IPv4 address
         GET_HOST_PREFIX,        // Gets host by IPv6 prefix
         GET_VERSION,            // Obtain version number
+        INSERT_HOST,            // Insert new host to collection
+        INSERT_V6_RESRV,        // Insert v6 reservation
+        INSERT_V4_OPTION,       // Insert DHCPv4 option
+        INSERT_V6_OPTION,       // Insert DHCPv6 option
         NUM_STATEMENTS          // Number of statements
     };
 
+    /// @brief Index of first statement performing write to the database.
+    ///
+    /// This value is used to mark border line between queries and other
+    /// statements and statements performing write operation on the database,
+    /// such as INSERT, DELETE, UPDATE.
+    static const StatementIndex WRITE_STMTS_BEGIN = INSERT_HOST;
+
     /// @brief Constructor.
     ///
     /// This constructor opens database connection and initializes prepared
@@ -1777,39 +1785,20 @@ public:
     /// @brief MySQL connection
     MySqlConnection conn_;
 
-};
-
+    /// @brief Indicates if the database is opened in read only mode.
+    bool is_readonly_;
 namespace {
-/// @brief Prepared MySQL statements used by the backend to insert and
-/// retrieve hosts from the database.
-TaggedStatement tagged_statements[] = {
-    // Inserts a host into the 'hosts' table.
-    {MySqlHostDataSourceImpl::INSERT_HOST,
-         "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, "
-            "dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, "
-            "dhcp4_client_classes, dhcp6_client_classes) "
-         "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"},
 
-    // Inserts a single IPv6 reservation into 'reservations' table.
-    {MySqlHostDataSourceImpl::INSERT_V6_RESRV,
-         "INSERT INTO ipv6_reservations(address, prefix_len, type, "
-            "dhcp6_iaid, host_id) "
-         "VALUES (?,?,?,?,?)"},
+};
 
-    // Inserts a single DHCPv4 option into 'dhcp4_options' table.
-    // Using fixed scope_id = 3, which associates an option with host.
-    {MySqlHostDataSourceImpl::INSERT_V4_OPTION,
-         "INSERT INTO dhcp4_options(option_id, code, value, formatted_value, space, "
-            "persistent, dhcp_client_class, dhcp4_subnet_id, host_id, scope_id) "
-         " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"},
 
-    // Inserts a single DHCPv6 option into 'dhcp6_options' table.
-    // Using fixed scope_id = 3, which associates an option with host.
-    {MySqlHostDataSourceImpl::INSERT_V6_OPTION,
-         "INSERT INTO dhcp6_options(option_id, code, value, formatted_value, space, "
-            "persistent, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) "
-         " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"},
+/// @brief Array of tagged statements.
+typedef boost::array<TaggedStatement, MySqlHostDataSourceImpl::NUM_STATEMENTS>
+TaggedStatementArray;
 
+/// @brief Prepared MySQL statements used by the backend to insert and
+/// retrieve hosts from the database.
+TaggedStatementArray tagged_statements = { {
     // Retrieves host information, IPv6 reservations and both DHCPv4 and
     // DHCPv6 options associated with the host. The LEFT JOIN clause is used
     // to retrieve information from 4 different tables using a single query.
@@ -1931,8 +1920,32 @@ TaggedStatement tagged_statements[] = {
     {MySqlHostDataSourceImpl::GET_VERSION,
             "SELECT version, minor FROM schema_version"},
 
-    // Marks the end of the statements table.
-    {MySqlHostDataSourceImpl::NUM_STATEMENTS, NULL}
+    // Inserts a host into the 'hosts' table.
+    {MySqlHostDataSourceImpl::INSERT_HOST,
+         "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, "
+            "dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, "
+            "dhcp4_client_classes, dhcp6_client_classes) "
+         "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"},
+
+    // Inserts a single IPv6 reservation into 'reservations' table.
+    {MySqlHostDataSourceImpl::INSERT_V6_RESRV,
+         "INSERT INTO ipv6_reservations(address, prefix_len, type, "
+            "dhcp6_iaid, host_id) "
+         "VALUES (?,?,?,?,?)"},
+
+    // Inserts a single DHCPv4 option into 'dhcp4_options' table.
+    // Using fixed scope_id = 3, which associates an option with host.
+    {MySqlHostDataSourceImpl::INSERT_V4_OPTION,
+         "INSERT INTO dhcp4_options(option_id, code, value, formatted_value, space, "
+            "persistent, dhcp_client_class, dhcp4_subnet_id, host_id, scope_id) "
+         " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"},
+
+    // Inserts a single DHCPv6 option into 'dhcp6_options' table.
+    // Using fixed scope_id = 3, which associates an option with host.
+    {MySqlHostDataSourceImpl::INSERT_V6_OPTION,
+         "INSERT INTO dhcp6_options(option_id, code, value, formatted_value, space, "
+            "persistent, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) "
+         " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}}
 };
 
 }; // end anonymouse namespace
@@ -1945,23 +1958,56 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters)
                                                      DHCP4_AND_DHCP6)),
       host_ipv6_reservation_exchange_(new MySqlIPv6ReservationExchange()),
       host_option_exchange_(new MySqlOptionExchange()),
-      conn_(parameters) {
+      conn_(parameters),
+      is_readonly_(false) {
 
     // Open the database.
     conn_.openDatabase();
 
-    // Disable 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
-    // background every second.  Setting the parameter to that value will speed
-    // up the system, but at the risk of losing data if the system crashes.
-    my_bool result = mysql_autocommit(conn_.mysql_, 0);
+    // Enable autocommit. In case transaction is explicitly used, this
+    // setting will be overwritten for the transaction. However, there are
+    // cases when lack of autocommit could cause transactions to hang
+    // until commit or rollback is explicitly called. This already
+    // caused issues for some unit tests which were unable to cleanup
+    // the database after the test because of pending transactions.
+    // Use of autocommit will eliminate this problem.
+    my_bool result = mysql_autocommit(conn_.mysql_, 1);
     if (result != 0) {
         isc_throw(DbOperationError, mysql_error(conn_.mysql_));
     }
 
-    // Prepare all statements likely to be used.
-    conn_.prepareStatements(tagged_statements, NUM_STATEMENTS);
+    // Prepare query statements. Those are will be only used to retrieve
+    // information from the database, so they can be used even if the
+    // database is read only for the current user.
+    conn_.prepareStatements(tagged_statements.begin(),
+                            tagged_statements.begin() + WRITE_STMTS_BEGIN,
+                            WRITE_STMTS_BEGIN);
+
+    std::string readonly_value = "false";
+    try {
+        readonly_value = conn_.getParameter("readonly");
+        boost::algorithm::to_lower(readonly_value);
+    } catch (...) {
+        // Parameter "readonly" hasn't been specified so we simply use
+        // the default value of "false".
+    }
+
+    if (readonly_value == "true" || readonly_value == "1") {
+        is_readonly_ = true;
+
+    } else if ((readonly_value != "false") && (readonly_value != "0")) {
+        isc_throw(BadValue, "invalid value '" << readonly_value
+                  << "' specified for boolean parameter 'readonly'");
+    }
+
+    // If we are using read-write mode for the database we also prepare
+    // statements for INSERTS etc.
+    if (!is_readonly_) {
+        // Prepare statements for writing to the database, e.g. INSERT.
+        conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN,
+                                tagged_statements.end(),
+                                tagged_statements.size());
+    }
 }
 
 MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() {
@@ -2163,11 +2209,14 @@ getHost(const SubnetID& subnet_id,
 
 MySqlHostDataSource::
 MySqlHostDataSource(const MySqlConnection::ParameterMap& parameters)
-    : impl_(new MySqlHostDataSourceImpl(parameters)) {
+    : impl_(new MySqlHostDataSourceImpl(parameters),
+            "MySQL host database backend is configured to"
+            " operate in read only mode") {
+    impl_.allowConstOnly(impl_->is_readonly_);
 }
 
 MySqlHostDataSource::~MySqlHostDataSource() {
-    delete impl_;
+    delete impl_.getPtr();
 }
 
 void

+ 3 - 1
src/lib/dhcpsrv/mysql_host_data_source.h

@@ -8,7 +8,9 @@
 #define MYSQL_HOST_DATA_SOURCE_H
 
 #include <dhcpsrv/base_host_data_source.h>
+#include <dhcpsrv/db_exceptions.h>
 #include <dhcpsrv/mysql_connection.h>
+#include <util/pointer_util.h>
 
 namespace isc {
 namespace dhcp {
@@ -255,7 +257,7 @@ public:
 private:
 
     /// @brief Pointer to the implementation of the @ref MySqlHostDataSource.
-    MySqlHostDataSourceImpl* impl_; 
+    util::RestrictedConstPtr<MySqlHostDataSourceImpl, ReadOnlyDb> impl_;
 };
 
 }

+ 8 - 6
src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2016 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
@@ -13,6 +13,7 @@
 #include <dhcpsrv/mysql_lease_mgr.h>
 #include <dhcpsrv/mysql_connection.h>
 
+#include <boost/array.hpp>
 #include <boost/static_assert.hpp>
 #include <mysqld_error.h>
 
@@ -82,7 +83,8 @@ const size_t HOSTNAME_MAX_LEN = 255;
 /// colon separators.
 const size_t ADDRESS6_TEXT_MAX_LEN = 39;
 
-TaggedStatement tagged_statements[] = {
+boost::array<TaggedStatement, MySqlLeaseMgr::NUM_STATEMENTS>
+tagged_statements = { {
     {MySqlLeaseMgr::DELETE_LEASE4,
                     "DELETE FROM lease4 WHERE address = ?"},
     {MySqlLeaseMgr::DELETE_LEASE4_STATE_EXPIRED,
@@ -204,9 +206,8 @@ TaggedStatement tagged_statements[] = {
                         "prefix_len = ?, fqdn_fwd = ?, fqdn_rev = ?, "
                         "hostname = ?, hwaddr = ?, hwtype = ?, hwaddr_source = ?, "
                         "state = ? "
-                            "WHERE address = ?"},
-    // End of list sentinel
-    {MySqlLeaseMgr::NUM_STATEMENTS, NULL}
+                            "WHERE address = ?"}
+    }
 };
 
 };
@@ -1236,7 +1237,8 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters)
     }
 
     // Prepare all statements likely to be used.
-    conn_.prepareStatements(tagged_statements, MySqlLeaseMgr::NUM_STATEMENTS);
+    conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end(),
+                            tagged_statements.size());
 
     // Create the exchange objects for use in exchanging data between the
     // program and the database.

+ 10 - 2
src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc

@@ -514,6 +514,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) {
     // The database is initially opened in "read-write" mode. We can
     // insert some data to the databse.
     HostPtr host = initializeHost6("2001:db8::1", Host::IDENT_DUID, false);
+    ASSERT_TRUE(host);
     ASSERT_NO_THROW(hdsptr_->add(host));
 
     // Subnet id will be used in queries to the database.
@@ -524,6 +525,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) {
     ConstHostPtr host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(),
                                             &host->getIdentifier()[0],
                                             host->getIdentifier().size());
+    ASSERT_TRUE(host_by_id);
     ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id));
 
     // Close the database connection and reopen in "read-only" mode as
@@ -536,15 +538,21 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) {
                                                    VALID_PASSWORD,
                                                    VALID_READONLY_DB));
 
+    hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr();
+
     // Check that an attempt to insert new host would result in
     // exception.
-    host = initializeHost6("2001:db8::2", Host::IDENT_DUID, false);
-    ASSERT_THROW(hdsptr_->add(host), ReadOnlyDb);
+    HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, false);
+    ASSERT_TRUE(host2);
+    ASSERT_THROW(hdsptr_->add(host2), ReadOnlyDb);
+    ASSERT_THROW(hdsptr_->commit(), ReadOnlyDb);
+    ASSERT_THROW(hdsptr_->rollback(), ReadOnlyDb);
 
     // Reading from the database should still be possible, though.
     host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(),
                                &host->getIdentifier()[0],
                                host->getIdentifier().size());
+    ASSERT_TRUE(host_by_id);
     ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id));
 }
 

+ 6 - 1
src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc

@@ -63,8 +63,13 @@ public:
     /// Rolls back all pending transactions.  The deletion of myhdsptr_ will close
     /// the database.  Then reopen it and delete everything created by the test.
     virtual ~MySqlHostDataSourceTest() {
-        hdsptr_->rollback();
+        try {
+            hdsptr_->rollback();
+        } catch (...) {
+            // Rollback may fail if backend is in read only mode. That's ok.
+        }
         HostDataSourceFactory::destroy();
+        hdsptr_.reset();
         destroyMySQLSchema();
     }
 

+ 41 - 1
src/lib/util/pointer_util.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 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
@@ -7,9 +7,49 @@
 #ifndef POINTER_UTIL_H
 #define POINTER_UTIL_H
 
+#include <exceptions/exceptions.h>
+#include <string>
+
 namespace isc {
 namespace util {
 
+template <typename T, typename E>
+class RestrictedConstPtr {
+public:
+
+    RestrictedConstPtr(T* ptr, const std::string& error_text)
+        : ptr_(ptr), const_only_(false), error_text_(error_text) {
+    }
+
+    void allowConstOnly(const bool const_only) {
+        const_only_ = const_only;
+    }
+
+    T* operator->() const {
+        return (ptr_);
+    }
+
+    T* operator->() {
+        if (const_only_) {
+            isc_throw(E, error_text_);
+        }
+        return (ptr_);
+    }
+
+    T* getPtr() const {
+        return (ptr_);
+    }
+
+private:
+
+    T* ptr_;
+
+    bool const_only_;
+
+    std::string error_text_;
+};
+
+
 /// @brief This function checks if two pointers are non-null and values
 /// are equal.
 ///