From fd42eccbfc7cb5065eb5a54b64da633cd429f13f Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Mon, 28 Oct 2019 18:10:55 +0200 Subject: [PATCH] [#887,!572] implement mysql thread handle --- src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc | 6 +- src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc | 8 +- src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc | 23 +-- src/lib/dhcpsrv/mysql_host_data_source.cc | 111 +++++----- src/lib/dhcpsrv/mysql_lease_mgr.cc | 120 +++++------ .../tests/mysql_host_data_source_unittest.cc | 18 +- .../mysql_generic_backend_unittest.cc | 8 +- src/lib/mysql/mysql_connection.cc | 189 ++++++++++++------ src/lib/mysql/mysql_connection.h | 159 +++++++++------ .../mysql/tests/mysql_connection_unittest.cc | 15 +- 10 files changed, 374 insertions(+), 283 deletions(-) diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index bec9a1ccf0..8541bf51a5 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -220,7 +220,7 @@ public: in_bindings); // Let's first get the primary key of the global parameter. - uint64_t id = mysql_insert_id(conn_.mysql_); + uint64_t id = mysql_insert_id(conn_.handle()); // Successfully inserted global parameter. Now, we have to associate it // with the server tag. @@ -1046,7 +1046,7 @@ public: // Run INSERT. conn_.insertQuery(INSERT_POOL4, in_bindings); - uint64_t pool_id = mysql_insert_id(conn_.mysql_); + uint64_t pool_id = mysql_insert_id(conn_.handle()); auto option_spaces = pool->getCfgOption()->getOptionSpaceNames(); for (auto option_space : option_spaces) { OptionContainerPtr options = pool->getCfgOption()->getAll(option_space); @@ -1577,7 +1577,7 @@ public: // Fetch primary key value of the inserted option. We will use it in the // next INSERT statement to associate this option with the server. - auto option_id = mysql_insert_id(conn_.mysql_); + auto option_id = mysql_insert_id(conn_.handle()); // Timestamp is expected to be in this input binding. auto timestamp_binding = in_bindings[11]; diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index 14ffaf363c..c809939195 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -231,7 +231,7 @@ public: // with the server tag. // Let's first get the primary key of the global parameter. - uint64_t id = mysql_insert_id(conn_.mysql_); + uint64_t id = mysql_insert_id(conn_.handle()); // Successfully inserted global parameter. Now, we have to associate it // with the server tag. @@ -1300,7 +1300,7 @@ public: // Run INSERT. conn_.insertQuery(INSERT_POOL6, in_bindings); - uint64_t pool_id = mysql_insert_id(conn_.mysql_); + uint64_t pool_id = mysql_insert_id(conn_.handle()); auto option_spaces = pool->getCfgOption()->getOptionSpaceNames(); for (auto option_space : option_spaces) { OptionContainerPtr options = pool->getCfgOption()->getAll(option_space); @@ -1351,7 +1351,7 @@ public: // Run INSERT. conn_.insertQuery(INSERT_PD_POOL, in_bindings); - uint64_t pd_pool_id = mysql_insert_id(conn_.mysql_); + uint64_t pd_pool_id = mysql_insert_id(conn_.handle()); auto option_spaces = pd_pool->getCfgOption()->getOptionSpaceNames(); for (auto option_space : option_spaces) { OptionContainerPtr options = pd_pool->getCfgOption()->getAll(option_space); @@ -1912,7 +1912,7 @@ public: // Fetch primary key value of the inserted option. We will use it in the // next INSERT statement to associate this option with the server. - auto option_id = mysql_insert_id(conn_.mysql_); + auto option_id = mysql_insert_id(conn_.handle()); // Timestamp is expected to be in this input binding. auto timestamp_binding = in_bindings[11]; diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index b3a9a32a5e..d082f8e20e 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -57,30 +57,9 @@ MySqlConfigBackendImpl(const DatabaseConnection::ParameterMap& parameters) << " found version: " << db_version.first << "." << db_version.second); } */ - - // 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 != MLM_FALSE) { - isc_throw(DbOperationError, mysql_error(conn_.mysql_)); - } } MySqlConfigBackendImpl::~MySqlConfigBackendImpl() { - // Free up the prepared statements, ignoring errors. (What would we do - // about them? We're destroying this object and are not really concerned - // with errors on a database connection that is about to go away.) - for (int i = 0; i < conn_.statements_.size(); ++i) { - if (conn_.statements_[i] != NULL) { - (void) mysql_stmt_close(conn_.statements_[i]); - conn_.statements_[i] = NULL; - } - } } MySqlBindingPtr @@ -570,7 +549,7 @@ MySqlConfigBackendImpl::createUpdateOptionDef(const db::ServerSelector& server_s // Fetch unique identifier of the inserted option definition and use it // as input to the next query. - uint64_t id = mysql_insert_id(conn_.mysql_); + uint64_t id = mysql_insert_id(conn_.handle()); // Insert associations of the option definition with servers. attachElementToServers(insert_option_def_server, diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 5340070881..ffad465634 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1027,10 +1027,26 @@ private: // processing. most_recent_option_id_ = 0; + option_id_ = 0; + code_ = 0; + persistent_ = false; + option_id_null_ = MLM_FALSE; + code_null_ = MLM_FALSE; + value_null_ = MLM_FALSE; + formatted_value_null_ = MLM_FALSE; + space_null_ = MLM_FALSE; + user_context_null_ = MLM_FALSE; + + memset(value_, 0, sizeof(value_)); + memset(formatted_value_, 0, sizeof(formatted_value_)); + memset(space_, 0, sizeof(space_)); + memset(user_context_, 0, sizeof(user_context_)); + // option_id : INT UNSIGNED NOT NULL AUTO_INCREMENT, bind[option_id_index_].buffer_type = MYSQL_TYPE_LONG; bind[option_id_index_].buffer = reinterpret_cast(&option_id_); bind[option_id_index_].is_unsigned = MLM_TRUE; + bind[option_id_index_].is_null = &option_id_null_; // code : TINYINT OR SHORT UNSIGNED NOT NULL bind[code_index_].buffer_type = MYSQL_TYPE_SHORT; @@ -2255,7 +2271,6 @@ TaggedStatementArray tagged_statements = { { "h.dhcp_identifier_type, h.dhcp4_subnet_id, " "h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " "h.dhcp4_client_classes, h.dhcp6_client_classes, h.user_context, " - "h.dhcp4_next_server, h.dhcp4_server_hostname, " "h.dhcp4_boot_file_name, h.auth_key, " "o.option_id, o.code, o.value, o.formatted_value, o.space, " @@ -2296,7 +2311,6 @@ TaggedStatementArray tagged_statements = { { "h.dhcp_identifier_type, h.dhcp4_subnet_id, " "h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " "h.dhcp4_client_classes, h.dhcp6_client_classes, h.user_context, " - "h.dhcp4_next_server, h.dhcp4_server_hostname, " "h.dhcp4_boot_file_name, h.auth_key, " "o.option_id, o.code, o.value, o.formatted_value, o.space, " @@ -2409,7 +2423,6 @@ TaggedStatementArray tagged_statements = { { "h.dhcp_identifier_type, h.dhcp4_subnet_id, " "h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " "h.dhcp4_client_classes, h.dhcp6_client_classes, h.user_context, " - "h.dhcp4_next_server, h.dhcp4_server_hostname, " "h.dhcp4_boot_file_name, h.auth_key, " "o.option_id, o.code, o.value, o.formatted_value, o.space, " @@ -2498,18 +2511,6 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) << db_version.second); } - // 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 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. @@ -2532,18 +2533,6 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) } MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() { - // Free up the prepared statements, ignoring errors. (What would we do - // about them? We're destroying this object and are not really concerned - // with errors on a database connection that is about to go away.) - for (int i = 0; i < conn_.statements_.size(); ++i) { - if (conn_.statements_[i] != NULL) { - (void) mysql_stmt_close(conn_.statements_[i]); - conn_.statements_[i] = NULL; - } - } - - // There is no need to close the database in this destructor: it is - // closed in the destructor of the mysql_ member variable. } std::pair @@ -2551,25 +2540,29 @@ MySqlHostDataSourceImpl::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_HOST_DB_GET_VERSION); + MySqlHolder& holderHandle = conn_.handle(); + // Allocate a new statement. - MYSQL_STMT *stmt = mysql_stmt_init(conn_.mysql_); + MYSQL_STMT *stmt = mysql_stmt_init(holderHandle); if (stmt == NULL) { isc_throw(DbOperationError, "unable to allocate MySQL prepared " - "statement structure, reason: " << mysql_error(conn_.mysql_)); + "statement structure, reason: " << mysql_error(holderHandle)); } // Prepare the statement from SQL text. const char* version_sql = "SELECT version, minor FROM schema_version"; int status = mysql_stmt_prepare(stmt, version_sql, strlen(version_sql)); if (status != 0) { + mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to prepare MySQL statement <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Execute the prepared statement. if (mysql_stmt_execute(stmt) != 0) { + mysql_stmt_close(stmt); isc_throw(DbOperationError, "cannot execute schema version query <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Bind the output of the statement to the appropriate variables. @@ -2590,14 +2583,14 @@ MySqlHostDataSourceImpl::getVersion() const { if (mysql_stmt_bind_result(stmt, bind)) { isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Fetch the data. if (mysql_stmt_fetch(stmt)) { mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Discard the statement and its resources @@ -2606,21 +2599,21 @@ MySqlHostDataSourceImpl::getVersion() const { return (std::make_pair(major, minor)); } - void MySqlHostDataSourceImpl::addStatement(StatementIndex stindex, std::vector& bind) { + MySqlHolder& holderHandle = conn_.handle(); // Bind the parameters to the statement - int status = mysql_stmt_bind_param(conn_.statements_[stindex], &bind[0]); + int status = mysql_stmt_bind_param(holderHandle.statements_[stindex], &bind[0]); checkError(status, stindex, "unable to bind parameters"); // Execute the statement - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); if (status != 0) { // Failure: check for the special case of duplicate entry. - if (mysql_errno(conn_.mysql_) == ER_DUP_ENTRY) { + if (mysql_errno(holderHandle) == ER_DUP_ENTRY) { isc_throw(DuplicateEntry, "Database duplicate entry error"); } checkError(status, stindex, "unable to execute"); @@ -2630,19 +2623,21 @@ MySqlHostDataSourceImpl::addStatement(StatementIndex stindex, bool MySqlHostDataSourceImpl::delStatement(StatementIndex stindex, MYSQL_BIND* bind) { + MySqlHolder& holderHandle = conn_.handle(); + // Bind the parameters to the statement - int status = mysql_stmt_bind_param(conn_.statements_[stindex], &bind[0]); + int status = mysql_stmt_bind_param(holderHandle.statements_[stindex], &bind[0]); checkError(status, stindex, "unable to bind parameters"); // Execute the statement - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); if (status != 0) { checkError(status, stindex, "unable to execute"); } // Let's check how many hosts were deleted. - my_ulonglong numrows = mysql_stmt_affected_rows(conn_.statements_[stindex]); + my_ulonglong numrows = mysql_stmt_affected_rows(holderHandle.statements_[stindex]); return (numrows != 0); } @@ -2706,32 +2701,33 @@ MySqlHostDataSourceImpl:: getHostCollection(StatementIndex stindex, MYSQL_BIND* bind, boost::shared_ptr exchange, ConstHostCollection& result, bool single) const { + MySqlHolder& holderHandle = conn_.handle(); // Bind the selection parameters to the statement - int status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); + int status = mysql_stmt_bind_param(holderHandle.statements_[stindex], bind); checkError(status, stindex, "unable to bind WHERE clause parameter"); // Set up the MYSQL_BIND array for the data being returned and bind it to // the statement. std::vector outbind = exchange->createBindForReceive(); - status = mysql_stmt_bind_result(conn_.statements_[stindex], &outbind[0]); + status = mysql_stmt_bind_result(holderHandle.statements_[stindex], &outbind[0]); checkError(status, stindex, "unable to bind SELECT clause parameters"); // Execute the statement - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); checkError(status, stindex, "unable to execute"); // Ensure that all the lease information is retrieved in one go to avoid // overhead of going back and forth between client and server. - status = mysql_stmt_store_result(conn_.statements_[stindex]); + status = mysql_stmt_store_result(holderHandle.statements_[stindex]); checkError(status, stindex, "unable to set up for storing all results"); // Set up the fetch "release" object to release resources associated // with the call to mysql_stmt_fetch when this method exits, then // retrieve the data. mysql_stmt_fetch return value equal to 0 represents // successful data fetch. - MySqlFreeResult fetch_release(conn_.statements_[stindex]); - while ((status = mysql_stmt_fetch(conn_.statements_[stindex])) == + MySqlFreeResult fetch_release(holderHandle.statements_[stindex]); + while ((status = mysql_stmt_fetch(holderHandle.statements_[stindex])) == MLM_MYSQL_FETCH_SUCCESS) { try { exchange->processFetchedData(result); @@ -2801,8 +2797,9 @@ getHost(const SubnetID& subnet_id, // Return single record if present, else clear the host. ConstHostPtr result; - if (!collection.empty()) + if (!collection.empty()) { result = *collection.begin(); + } return (result); } @@ -2826,6 +2823,8 @@ MySqlHostDataSource::~MySqlHostDataSource() { void MySqlHostDataSource::add(const HostPtr& host) { + MySqlHolder& holderHandle = impl_->conn_.handle(); + // If operating in read-only mode, throw exception. impl_->checkReadOnly(); @@ -2842,7 +2841,7 @@ MySqlHostDataSource::add(const HostPtr& host) { impl_->addStatement(MySqlHostDataSourceImpl::INSERT_HOST, bind); // Gets the last inserted hosts id - uint64_t host_id = mysql_insert_id(impl_->conn_.mysql_); + uint64_t host_id = mysql_insert_id(holderHandle); // Insert DHCPv4 options. ConstCfgOptionPtr cfg_option4 = host->getCfgOption4(); @@ -2896,7 +2895,7 @@ MySqlHostDataSource::del(const SubnetID& subnet_id, const asiolink::IOAddress& a } // v6 - ConstHostPtr host = get6(subnet_id, addr); + ConstHostPtr host(get6(subnet_id, addr)); if (!host) { return (false); } @@ -3223,10 +3222,9 @@ MySqlHostDataSource::get4(const SubnetID& subnet_id, ConstHostPtr MySqlHostDataSource::get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const { - // Check that address is IPv4, not IPv6. if (!address.isV4()) { - isc_throw(BadValue, "MySqlHostDataSource::get4(2): wrong address type, " - "address supplied is not an IPv4 address"); + isc_throw(BadValue, "MySqlHostDataSource::get4(id, address): " + "wrong address type, address supplied is an IPv6 address"); } // Set up the WHERE clause value @@ -3248,8 +3246,9 @@ MySqlHostDataSource::get4(const SubnetID& subnet_id, // Return single record if present, else clear the host. ConstHostPtr result; - if (!collection.empty()) + if (!collection.empty()) { result = *collection.begin(); + } return (result); } @@ -3339,7 +3338,8 @@ MySqlHostDataSource::get6(const SubnetID& subnet_id, // Miscellaneous database methods. -std::string MySqlHostDataSource::getName() const { +std::string +MySqlHostDataSource::getName() const { std::string name = ""; try { name = impl_->conn_.getParameter("name"); @@ -3349,7 +3349,8 @@ std::string MySqlHostDataSource::getName() const { return (name); } -std::string MySqlHostDataSource::getDescription() const { +std::string +MySqlHostDataSource::getDescription() const { return (std::string("Host data source that stores host information" "in MySQL database")); } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 599045c860..f55cf0e0bf 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -307,19 +307,19 @@ tagged_statements = { { {MySqlLeaseMgr::ALL_LEASE6_STATS, "SELECT subnet_id, lease_type, state, leases as state_count" - " FROM lease6_stat ORDER BY subnet_id, lease_type, state" }, + " FROM lease6_stat ORDER BY subnet_id, lease_type, state"}, {MySqlLeaseMgr::SUBNET_LEASE6_STATS, "SELECT subnet_id, lease_type, state, leases as state_count" " FROM lease6_stat " " WHERE subnet_id = ? " - " ORDER BY lease_type, state" }, + " ORDER BY lease_type, state"}, {MySqlLeaseMgr::SUBNET_RANGE_LEASE6_STATS, "SELECT subnet_id, lease_type, state, leases as state_count" " FROM lease6_stat " " WHERE subnet_id >= ? and subnet_id <= ? " - " ORDER BY subnet_id, lease_type, state" } + " ORDER BY subnet_id, lease_type, state"} } }; @@ -529,7 +529,7 @@ public: bind_[2].buffer_length = client_id_length_; bind_[2].length = &client_id_length_; // bind_[2].is_null = &MLM_FALSE; // commented out for performance - // reasons, see memset() above + // reasons, see memset() above } else { bind_[2].buffer_type = MYSQL_TYPE_NULL; // According to http://dev.mysql.com/doc/refman/5.5/en/ @@ -595,7 +595,7 @@ public: // bind_[8].is_null = &MLM_FALSE; // commented out for performance // reasons, see memset() above - // state: uint32_t. + // state: uint32_t bind_[9].buffer_type = MYSQL_TYPE_LONG; bind_[9].buffer = reinterpret_cast(&lease_->state_); bind_[9].is_unsigned = MLM_TRUE; @@ -721,7 +721,7 @@ public: // bind_[8].is_null = &MLM_FALSE; // commented out for performance // reasons, see memset() above - // state: uint32_t + // state: uint32_t bind_[9].buffer_type = MYSQL_TYPE_LONG; bind_[9].buffer = reinterpret_cast(&state_); bind_[9].is_unsigned = MLM_TRUE; @@ -1126,7 +1126,7 @@ public: bind_[14].is_null = &hwaddr_null_; } - // state: uint32_t + // state: uint32_t bind_[15].buffer_type = MYSQL_TYPE_LONG; bind_[15].buffer = reinterpret_cast(&lease_->state_); bind_[15].is_unsigned = MLM_TRUE; @@ -1293,7 +1293,7 @@ public: bind_[14].buffer = reinterpret_cast(&hwaddr_source_); bind_[14].is_unsigned = MLM_TRUE; - // state: uint32_t + // state: uint32_t bind_[15].buffer_type = MYSQL_TYPE_LONG; bind_[15].buffer = reinterpret_cast(&state_); bind_[15].is_unsigned = MLM_TRUE; @@ -1489,7 +1489,7 @@ public: // Set the number of columns in the bind array based on fetch_type // This is the number of columns expected in the result set bind_(fetch_type_ ? 4 : 3), - subnet_id_(0), lease_type_(0), lease_state_(0), state_count_(0) { + subnet_id_(0), lease_type_(0), state_(0), state_count_(0) { validateStatement(); } @@ -1509,7 +1509,7 @@ public: // Set the number of columns in the bind array based on fetch_type // This is the number of columns expected in the result set bind_(fetch_type_ ? 4 : 3), - subnet_id_(0), lease_type_(0), lease_state_(0), state_count_(0) { + subnet_id_(0), lease_type_(0), state_(0), state_count_(0) { validateStatement(); } @@ -1533,7 +1533,7 @@ public: // Set the number of columns in the bind array based on fetch_type // This is the number of columns expected in the result set bind_(fetch_type_ ? 4 : 3), - subnet_id_(0), lease_type_(0), lease_state_(0), state_count_(0) { + subnet_id_(0), lease_type_(0), state_(0), state_count_(0) { validateStatement(); } @@ -1592,7 +1592,7 @@ public: // state: uint32_t bind_[col].buffer_type = MYSQL_TYPE_LONG; - bind_[col].buffer = reinterpret_cast(&lease_state_); + bind_[col].buffer = reinterpret_cast(&state_); bind_[col].is_unsigned = MLM_TRUE; ++col; @@ -1633,7 +1633,7 @@ public: if (status == MLM_MYSQL_FETCH_SUCCESS) { row.subnet_id_ = static_cast(subnet_id_); row.lease_type_ = static_cast(lease_type_); - row.lease_state_ = lease_state_; + row.lease_state_ = state_; row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { @@ -1648,12 +1648,14 @@ private: /// Safely fetch the statement from the connection based on statement index /// @throw BadValue if statement index is out of range void validateStatement() { + MySqlHolder& holderHandle = conn_.handle(); + if (statement_index_ >= MySqlLeaseMgr::NUM_STATEMENTS) { isc_throw(BadValue, "MySqlLeaseStatsQuery" " - invalid statement index" << statement_index_); } - statement_ = conn_.statements_[statement_index_]; + statement_ = holderHandle.statements_[statement_index_]; } /// @brief Database connection to use to execute the query @@ -1676,15 +1678,15 @@ private: /// @brief Receives the lease type when fetching a row uint32_t lease_type_; /// @brief Receives the lease state when fetching a row - uint32_t lease_state_; + uint32_t state_; /// @brief Receives the state count when fetching a row int64_t state_count_; }; // MySqlLeaseMgr Constructor and Destructor -MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) - : conn_(parameters) { +MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) : + conn_(parameters) { // Open the database. conn_.openDatabase(); @@ -1694,21 +1696,10 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) MYSQL_SCHEMA_VERSION_MINOR); std::pair 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); - } - - // 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 - // 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_, 1); - if (result != 0) { - isc_throw(DbOperationError, mysql_error(conn_.mysql_)); + isc_throw(DbOpenError, "MySQL schema version mismatch: need version: " + << code_version.first << "." << code_version.second + << " found version: " << db_version.first << "." + << db_version.second); } // Prepare all statements likely to be used. @@ -1741,19 +1732,20 @@ MySqlLeaseMgr::getDBVersion() { bool MySqlLeaseMgr::addLeaseCommon(StatementIndex stindex, std::vector& bind) { + MySqlHolder& holderHandle = conn_.handle(); // Bind the parameters to the statement - int status = mysql_stmt_bind_param(conn_.statements_[stindex], &bind[0]); + int status = mysql_stmt_bind_param(holderHandle.statements_[stindex], &bind[0]); checkError(status, stindex, "unable to bind parameters"); // Execute the statement - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); if (status != 0) { // Failure: check for the special case of duplicate entry. If this is // the case, we return false to indicate that the row was not added. // Otherwise we throw an exception. - if (mysql_errno(conn_.mysql_) == ER_DUP_ENTRY) { + if (mysql_errno(holderHandle) == ER_DUP_ENTRY) { return (false); } checkError(status, stindex, "unable to execute"); @@ -1819,36 +1811,36 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, Exchange& exchange, LeaseCollection& result, bool single) const { - + MySqlHolder& holderHandle = conn_.handle(); int status; if (bind) { // Bind the selection parameters to the statement - status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); + status = mysql_stmt_bind_param(holderHandle.statements_[stindex], bind); checkError(status, stindex, "unable to bind WHERE clause parameter"); } // Set up the MYSQL_BIND array for the data being returned and bind it to // the statement. std::vector outbind = exchange->createBindForReceive(); - status = mysql_stmt_bind_result(conn_.statements_[stindex], &outbind[0]); + status = mysql_stmt_bind_result(holderHandle.statements_[stindex], &outbind[0]); checkError(status, stindex, "unable to bind SELECT clause parameters"); // Execute the statement - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); checkError(status, stindex, "unable to execute"); // Ensure that all the lease information is retrieved in one go to avoid // overhead of going back and forth between client and server. - status = mysql_stmt_store_result(conn_.statements_[stindex]); + status = mysql_stmt_store_result(holderHandle.statements_[stindex]); checkError(status, stindex, "unable to set up for storing all results"); // Set up the fetch "release" object to release resources associated // with the call to mysql_stmt_fetch when this method exits, then // retrieve the data. - MySqlFreeResult fetch_release(conn_.statements_[stindex]); + MySqlFreeResult fetch_release(holderHandle.statements_[stindex]); int count = 0; - while ((status = mysql_stmt_fetch(conn_.statements_[stindex])) == 0) { + while ((status = mysql_stmt_fetch(holderHandle.statements_[stindex])) == 0) { try { result.push_back(exchange->getLeaseData()); @@ -2495,18 +2487,19 @@ template void MySqlLeaseMgr::updateLeaseCommon(StatementIndex stindex, MYSQL_BIND* bind, const LeasePtr& lease) { + MySqlHolder& holderHandle = conn_.handle(); // Bind the parameters to the statement - int status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); + int status = mysql_stmt_bind_param(holderHandle.statements_[stindex], bind); checkError(status, stindex, "unable to bind parameters"); // Execute - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); checkError(status, stindex, "unable to execute"); // See how many rows were affected. The statement should only update a // single row. - int affected_rows = mysql_stmt_affected_rows(conn_.statements_[stindex]); + int affected_rows = mysql_stmt_affected_rows(holderHandle.statements_[stindex]); if (affected_rows == 0) { isc_throw(NoSuchLease, "unable to update lease for address " << lease->addr_ << " as it does not exist"); @@ -2579,18 +2572,19 @@ MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { uint64_t MySqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex, MYSQL_BIND* bind) { + MySqlHolder& holderHandle = conn_.handle(); // Bind the input parameters to the statement - int status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); + int status = mysql_stmt_bind_param(holderHandle.statements_[stindex], bind); checkError(status, stindex, "unable to bind WHERE clause parameter"); // Execute - status = mysql_stmt_execute(conn_.statements_[stindex]); + status = mysql_stmt_execute(holderHandle.statements_[stindex]); checkError(status, stindex, "unable to execute"); // See how many rows were affected. Note that the statement may delete // multiple rows. - return (static_cast(mysql_stmt_affected_rows(conn_.statements_[stindex]))); + return (static_cast(mysql_stmt_affected_rows(holderHandle.statements_[stindex]))); } bool @@ -2764,25 +2758,29 @@ MySqlLeaseMgr::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_GET_VERSION); + MySqlHolder& holderHandle = conn_.handle(); + // Allocate a new statement. - MYSQL_STMT *stmt = mysql_stmt_init(conn_.mysql_); + MYSQL_STMT *stmt = mysql_stmt_init(holderHandle); if (stmt == NULL) { isc_throw(DbOperationError, "unable to allocate MySQL prepared " - "statement structure, reason: " << mysql_error(conn_.mysql_)); + "statement structure, reason: " << mysql_error(holderHandle)); } // Prepare the statement from SQL text. const char* version_sql = "SELECT version, minor FROM schema_version"; int status = mysql_stmt_prepare(stmt, version_sql, strlen(version_sql)); if (status != 0) { + mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to prepare MySQL statement <" - << version_sql << ">, reason: " << mysql_error(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Execute the prepared statement. if (mysql_stmt_execute(stmt) != 0) { + mysql_stmt_close(stmt); isc_throw(DbOperationError, "cannot execute schema version query <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Bind the output of the statement to the appropriate variables. @@ -2803,14 +2801,14 @@ MySqlLeaseMgr::getVersion() const { if (mysql_stmt_bind_result(stmt, bind)) { isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Fetch the data. if (mysql_stmt_fetch(stmt)) { mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << version_sql << ">, reason: " << mysql_error(holderHandle)); } // Discard the statement and its resources @@ -2822,16 +2820,22 @@ MySqlLeaseMgr::getVersion() const { void MySqlLeaseMgr::commit() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_COMMIT); - if (mysql_commit(conn_.mysql_) != 0) { - isc_throw(DbOperationError, "commit failed: " << mysql_error(conn_.mysql_)); + + MySqlHolder& holderHandle = conn_.handle(); + + if (mysql_commit(holderHandle) != 0) { + isc_throw(DbOperationError, "commit failed: " << mysql_error(holderHandle)); } } void MySqlLeaseMgr::rollback() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_ROLLBACK); - if (mysql_rollback(conn_.mysql_) != 0) { - isc_throw(DbOperationError, "rollback failed: " << mysql_error(conn_.mysql_)); + + MySqlHolder& holderHandle = conn_.handle(); + + if (mysql_rollback(holderHandle) != 0) { + isc_throw(DbOperationError, "rollback failed: " << mysql_error(holderHandle)); } } diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 54f294ad51..c3f6c21fb4 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -120,12 +120,14 @@ public: MySqlConnection conn(params); conn.openDatabase(); - int status = mysql_query(conn.mysql_, query.c_str()); + MySqlHolder& holderHandle = conn.handle(); + + int status = mysql_query(holderHandle, query.c_str()); if (status !=0) { - isc_throw(DbOperationError, "Query failed: " << mysql_error(conn.mysql_)); + isc_throw(DbOperationError, "Query failed: " << mysql_error(holderHandle)); } - MYSQL_RES * res = mysql_store_result(conn.mysql_); + MYSQL_RES * res = mysql_store_result(holderHandle); int numrows = static_cast(mysql_num_rows(res)); mysql_free_result(res); @@ -658,9 +660,13 @@ TEST_F(MySqlHostDataSourceTest, testAddRollback) { MySqlConnection conn(params); ASSERT_NO_THROW(conn.openDatabase()); - int status = mysql_query(conn.mysql_, - "DROP TABLE IF EXISTS ipv6_reservations"); - ASSERT_EQ(0, status) << mysql_error(conn.mysql_); + MySqlHolder& holderHandle = conn.handle(); + + ConstHostCollection collection = hdsptr_->getAll4(0); + ASSERT_EQ(collection.size(), 0); + + int status = mysql_query(holderHandle, "DROP TABLE IF EXISTS ipv6_reservations"); + ASSERT_EQ(0, status) << mysql_error(holderHandle); // Create a host with a reservation. HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8:1::1", diff --git a/src/lib/dhcpsrv/testutils/mysql_generic_backend_unittest.cc b/src/lib/dhcpsrv/testutils/mysql_generic_backend_unittest.cc index b45aa7e1ea..44ee8bce54 100644 --- a/src/lib/dhcpsrv/testutils/mysql_generic_backend_unittest.cc +++ b/src/lib/dhcpsrv/testutils/mysql_generic_backend_unittest.cc @@ -19,16 +19,18 @@ MySqlGenericBackendTest::MySqlGenericBackendTest() size_t MySqlGenericBackendTest::countRows(MySqlConnection& conn, const std::string& table) const { + MySqlHolder& holderHandle = conn.handle(); + // Execute a simple select query on all rows. std::string query = "SELECT * FROM " + table; - auto status = mysql_query(conn.mysql_, query.c_str()); + auto status = mysql_query(holderHandle, query.c_str()); if (status != 0) { - ADD_FAILURE() << "Query failed: " << mysql_error(conn.mysql_); + ADD_FAILURE() << "Query failed: " << mysql_error(holderHandle); return (0); } // Get the number of rows returned and free the result. - MYSQL_RES * res = mysql_store_result(conn.mysql_); + MYSQL_RES * res = mysql_store_result(holderHandle); unsigned numrows = static_cast(mysql_num_rows(res)); mysql_free_result(res); diff --git a/src/lib/mysql/mysql_connection.cc b/src/lib/mysql/mysql_connection.cc index 8efc9a06ca..17a80004e4 100644 --- a/src/lib/mysql/mysql_connection.cc +++ b/src/lib/mysql/mysql_connection.cc @@ -23,7 +23,69 @@ using namespace std; namespace isc { namespace db { -bool MySqlHolder::atexit_ = false; +bool MySqlHolder::atexit_ = []{atexit([]{mysql_library_end();});return true;}; + +void +MySqlHolder::setConnection(MYSQL* connection) { + clearPrepared(); + if (mysql_ != NULL) { + mysql_close(mysql_); + } + mysql_ = connection; + connected_ = false; + prepared_ = false; +} + +void +MySqlHolder::clearPrepared() { + // Free up the prepared statements, ignoring errors. (What would we do + // about them? We're destroying this object and are not really concerned + // with errors on a database connection that is about to go away.) + for (int i = 0; i < statements_.size(); ++i) { + if (statements_[i] != NULL) { + (void) mysql_stmt_close(statements_[i]); + statements_[i] = NULL; + } + } + statements_.clear(); +} + +void +MySqlHolder::openDatabase(MySqlConnection& connection) { + if (connected_) { + return; + } + connected_ = true; + prepared_ = true; + connection.openDatabase(); + prepared_ = false; +} + +void +MySqlHolder::prepareStatements(MySqlConnection& connection) { + if (prepared_) { + return; + } + clearPrepared(); + statements_.resize(connection.text_statements_.size(), NULL); + uint32_t index = 0; + for (auto it = connection.text_statements_.begin(); + it != connection.text_statements_.end(); ++it) { + statements_[index] = mysql_stmt_init(mysql_); + if (statements_[index] == NULL) { + isc_throw(DbOperationError, "unable to allocate MySQL prepared " + "statement structure, reason: " << mysql_error(mysql_)); + } + + int status = mysql_stmt_prepare(statements_[index], it->c_str(), it->size()); + if (status != 0) { + isc_throw(DbOperationError, "unable to prepare MySQL statement <" << + *it << ">, reason: " << mysql_error(mysql_)); + } + ++index; + } + prepared_ = true; +} /// @todo: Migrate this default value to src/bin/dhcpX/simple_parserX.cc const int MYSQL_DEFAULT_CONNECTION_TIMEOUT = 5; // seconds @@ -37,7 +99,10 @@ MySqlTransaction::~MySqlTransaction() { // Rollback if the MySqlTransaction::commit wasn't explicitly // called. if (!committed_) { - conn_.rollback(); + try { + conn_.rollback(); + } catch (...) { + } } } @@ -126,10 +191,8 @@ MySqlConnection::openDatabase() { // No timeout parameter, we are going to use the default timeout. stimeout = ""; } - if (stimeout.size() > 0) { // Timeout was given, so try to convert it to an integer. - try { connect_timeout = boost::lexical_cast(stimeout); } catch (...) { @@ -162,18 +225,26 @@ MySqlConnection::openDatabase() { // connection after a reconnect as among other things, it drops all our // pre-compiled statements. my_bool auto_reconnect = MLM_FALSE; - int result = mysql_options(mysql_, MYSQL_OPT_RECONNECT, &auto_reconnect); + + MYSQL* new_conn = mysql_init(NULL); + if (new_conn == NULL) { + isc_throw(db::DbOpenError, "unable to initialize MySQL"); + } + + int result = mysql_options(new_conn, MYSQL_OPT_RECONNECT, &auto_reconnect); if (result != 0) { + mysql_close(new_conn); isc_throw(DbOpenError, "unable to set auto-reconnect option: " << - mysql_error(mysql_)); + mysql_error(new_conn)); } // Make sure we have a large idle time window ... say 30 days... const char *wait_time = "SET SESSION wait_timeout = 30 * 86400"; - result = mysql_options(mysql_, MYSQL_INIT_COMMAND, wait_time); + result = mysql_options(new_conn, MYSQL_INIT_COMMAND, wait_time); if (result != 0) { + mysql_close(new_conn); isc_throw(DbOpenError, "unable to set wait_timeout " << - mysql_error(mysql_)); + mysql_error(new_conn)); } // Set SQL mode options for the connection: SQL mode governs how what @@ -181,18 +252,20 @@ MySqlConnection::openDatabase() { // invalid data. We want to ensure we get the strictest behavior and // to reject invalid data with an error. const char *sql_mode = "SET SESSION sql_mode ='STRICT_ALL_TABLES'"; - result = mysql_options(mysql_, MYSQL_INIT_COMMAND, sql_mode); + result = mysql_options(new_conn, MYSQL_INIT_COMMAND, sql_mode); if (result != 0) { + mysql_close(new_conn); isc_throw(DbOpenError, "unable to set SQL mode options: " << - mysql_error(mysql_)); + mysql_error(new_conn)); } // Connection timeout, the amount of time taken for the client to drop // the connection if the server is not responding. - result = mysql_options(mysql_, MYSQL_OPT_CONNECT_TIMEOUT, &connect_timeout); + result = mysql_options(new_conn, MYSQL_OPT_CONNECT_TIMEOUT, &connect_timeout); if (result != 0) { + mysql_close(new_conn); isc_throw(DbOpenError, "unable to set database connection timeout: " << - mysql_error(mysql_)); + mysql_error(new_conn)); } // Open the database. @@ -205,11 +278,31 @@ MySqlConnection::openDatabase() { // This makes it hard to distinguish whether the UPDATE changed no rows // because no row matching the WHERE clause was found, or because a // row was found but no data was altered. - MYSQL* status = mysql_real_connect(mysql_, host, user, password, name, + MYSQL* status = mysql_real_connect(new_conn, host, user, password, name, port, NULL, CLIENT_FOUND_ROWS); - if (status != mysql_) { - isc_throw(DbOpenError, mysql_error(mysql_)); + if (status != new_conn) { + mysql_close(new_conn); + isc_throw(DbOpenError, mysql_error(new_conn)); } + + // 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 auto_commit = mysql_autocommit(new_conn, 1); + if (auto_commit != MLM_FALSE) { + mysql_close(new_conn); + isc_throw(DbOperationError, mysql_error(new_conn)); + } + + // We have a valid connection, so let's save it to our holder + MySqlHolder& holderHandle = handle(); + holderHandle.setConnection(new_conn); + holderHandle.connected_ = true; + connected_ = true; } @@ -224,25 +317,13 @@ void MySqlConnection::prepareStatement(uint32_t index, const char* text) { // Validate that there is space for the statement in the statements array // and that nothing has been placed there before. - if ((index >= statements_.size()) || (statements_[index] != NULL)) { + if (index >= text_statements_.size()) { isc_throw(InvalidParameter, "invalid prepared statement index (" << - static_cast(index) << ") or indexed prepared " << - "statement is not null"); + static_cast(index) << ")"); } // All OK, so prepare the statement text_statements_[index] = std::string(text); - statements_[index] = mysql_stmt_init(mysql_); - if (statements_[index] == NULL) { - isc_throw(DbOperationError, "unable to allocate MySQL prepared " - "statement structure, reason: " << mysql_error(mysql_)); - } - - int status = mysql_stmt_prepare(statements_[index], text, strlen(text)); - if (status != 0) { - isc_throw(DbOperationError, "unable to prepare MySQL statement <" << - text << ">, reason: " << mysql_error(mysql_)); - } } void @@ -251,34 +332,20 @@ MySqlConnection::prepareStatements(const TaggedStatement* start_statement, // Created the MySQL prepared statements for each DML statement. for (const TaggedStatement* tagged_statement = start_statement; tagged_statement != end_statement; ++tagged_statement) { - if (tagged_statement->index >= statements_.size()) { - statements_.resize(tagged_statement->index + 1, NULL); + if (tagged_statement->index >= text_statements_.size()) { text_statements_.resize(tagged_statement->index + 1, std::string("")); } prepareStatement(tagged_statement->index, tagged_statement->text); } -} - -void MySqlConnection::clearStatements() { - statements_.clear(); - text_statements_.clear(); + prepared_ = true; } /// @brief Destructor MySqlConnection::~MySqlConnection() { - // Free up the prepared statements, ignoring errors. (What would we do - // about them? We're destroying this object and are not really concerned - // with errors on a database connection that is about to go away.) - for (int i = 0; i < statements_.size(); ++i) { - if (statements_[i] != NULL) { - (void) mysql_stmt_close(statements_[i]); - statements_[i] = NULL; - } - } - statements_.clear(); text_statements_.clear(); + handle().clear(); } // Time conversion methods. @@ -299,8 +366,8 @@ MySqlConnection::convertToDatabaseTime(const time_t input_time, void MySqlConnection::convertToDatabaseTime(const time_t cltt, - const uint32_t valid_lifetime, - MYSQL_TIME& expire) { + const uint32_t valid_lifetime, + MYSQL_TIME& expire) { MySqlBinding::convertToDatabaseTime(cltt, valid_lifetime, expire); } @@ -315,31 +382,39 @@ MySqlConnection::startTransaction() { DB_LOG_DEBUG(DB_DBG_TRACE_DETAIL, MYSQL_START_TRANSACTION); // We create prepared statements for all other queries, but MySQL // don't support prepared statements for START TRANSACTION. - int status = mysql_query(mysql_, "START TRANSACTION"); + + MySqlHolder& holderHandle = handle(); + + int status = mysql_query(holderHandle, "START TRANSACTION"); if (status != 0) { isc_throw(DbOperationError, "unable to start transaction, " - "reason: " << mysql_error(mysql_)); + "reason: " << mysql_error(holderHandle)); } } void MySqlConnection::commit() { DB_LOG_DEBUG(DB_DBG_TRACE_DETAIL, MYSQL_COMMIT); - if (mysql_commit(mysql_) != 0) { + + MySqlHolder& holderHandle = handle(); + + if (mysql_commit(holderHandle) != 0) { isc_throw(DbOperationError, "commit failed: " - << mysql_error(mysql_)); + << mysql_error(holderHandle)); } } void MySqlConnection::rollback() { DB_LOG_DEBUG(DB_DBG_TRACE_DETAIL, MYSQL_ROLLBACK); - if (mysql_rollback(mysql_) != 0) { + + MySqlHolder& holderHandle = handle(); + + if (mysql_rollback(holderHandle) != 0) { isc_throw(DbOperationError, "rollback failed: " - << mysql_error(mysql_)); + << mysql_error(holderHandle)); } } - -} // namespace isc::db -} // namespace isc +} // namespace db +} // namespace isc diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h index 929fbc16e5..41da43ff31 100644 --- a/src/lib/mysql/mysql_connection.h +++ b/src/lib/mysql/mysql_connection.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -24,7 +25,6 @@ namespace isc { namespace db { - /// @brief Fetch and Release MySQL Results /// /// When a MySQL statement is expected, to fetch the results the function @@ -41,7 +41,6 @@ namespace db { class MySqlFreeResult { public: - /// @brief Constructor /// /// Store the pointer to the statement for which data is being fetched. @@ -52,18 +51,18 @@ public: /// way, any error from mysql_stmt_free_result is ignored. (Generating /// an exception is not much help, as it will only confuse things if the /// method calling mysql_stmt_fetch is exiting via an exception.) - MySqlFreeResult(MYSQL_STMT* statement) : statement_(statement) - {} + MySqlFreeResult(MYSQL_STMT* statement) : statement_(statement) { + } /// @brief Destructor /// /// Frees up fetch context if a fetch has been successfully executed. ~MySqlFreeResult() { - (void) mysql_stmt_free_result(statement_); + (void)mysql_stmt_free_result(statement_); } private: - MYSQL_STMT* statement_; ///< Statement for which results are freed + MYSQL_STMT* statement_; ///< Statement for which results are freed }; /// @brief MySQL Selection Statements @@ -76,6 +75,10 @@ struct TaggedStatement { const char* text; }; + +/// @brief Forward declaration to @ref MySqlConnection. +class MySqlConnection; + /// @brief MySQL Handle Holder /// /// Small RAII object for safer initialization, will close the database @@ -89,33 +92,28 @@ struct TaggedStatement { /// For this reason, the class is declared noncopyable. class MySqlHolder : public boost::noncopyable { public: - /// @brief Constructor /// /// Push a call to mysql_library_end() at exit time. /// Initialize MySql and store the associated context object. /// /// @throw DbOpenError Unable to initialize MySql handle. - MySqlHolder() : mysql_(mysql_init(NULL)) { - if (!atexit_) { - atexit([]{ mysql_library_end(); }); - atexit_ = true; - } - if (mysql_ == NULL) { - isc_throw(db::DbOpenError, "unable to initialize MySQL"); - } + MySqlHolder() : connected_(false), prepared_(false), mysql_(NULL) { } /// @brief Destructor /// /// Frees up resources allocated by the initialization of MySql. ~MySqlHolder() { - if (mysql_ != NULL) { - mysql_close(mysql_); - } + clear(); // @note Moved the call to mysql_library_end() to atexit. } + /// @brief Sets the connection to the value given + /// + /// @param connection - pointer to the MYSQL connection instance + void setConnection(MYSQL* connection); + /// @brief Conversion Operator /// /// Allows the MySqlHolder object to be passed as the context argument to @@ -124,15 +122,32 @@ public: return (mysql_); } + void clear() { + setConnection(NULL); + } + + void clearPrepared(); + + void openDatabase(MySqlConnection& connection); + + void prepareStatements(MySqlConnection& connection); + + /// @brief Prepared statements + /// + /// This field is public, because it is used heavily from MySqlConnection + /// and from MySqlHostDataSource. + std::vector statements_; + + bool connected_; ///< Flag to indicate openDatabase has been called + private: + bool prepared_; ///< Flag to indicate prepareStatements has been called + static bool atexit_; ///< Flag to call atexit once. MYSQL* mysql_; ///< Initialization context }; -/// @brief Forward declaration to @ref MySqlConnection. -class MySqlConnection; - /// @brief RAII object representing MySQL transaction. /// /// An instance of this class should be created in a scope where multiple @@ -155,7 +170,6 @@ class MySqlConnection; /// database which don't use transactions will still be auto committed. class MySqlTransaction : public boost::noncopyable { public: - /// @brief Constructor. /// /// Starts transaction by making a "START TRANSACTION" query. @@ -175,7 +189,6 @@ public: void commit(); private: - /// @brief Holds reference to the MySQL database connection. MySqlConnection& conn_; @@ -186,7 +199,6 @@ private: bool committed_; }; - /// @brief Common MySQL Connector Pool /// /// This class provides common operations for MySQL database connection @@ -196,15 +208,14 @@ private: /// that use instances of MySqlConnection. class MySqlConnection : public db::DatabaseConnection { public: - /// @brief Function invoked to process fetched row. typedef std::function ConsumeResultFun; /// @brief Constructor /// /// Initialize MySqlConnection object with parameters needed for connection. - MySqlConnection(const ParameterMap& parameters) - : DatabaseConnection(parameters) { + MySqlConnection(const ParameterMap& parameters) : + DatabaseConnection(parameters), connected_(false), prepared_(false) { } /// @brief Destructor @@ -242,9 +253,6 @@ public: void prepareStatements(const TaggedStatement* start_statement, const TaggedStatement* end_statement); - /// @brief Clears prepared statements and text statements. - void clearStatements(); - /// @brief Open Database /// /// Opens the database using the information supplied in the parameters @@ -350,6 +358,8 @@ public: const MySqlBindingCollection& in_bindings, MySqlBindingCollection& out_bindings, ConsumeResultFun process_result) { + MySqlHolder& holderHandle = handle(); + // Extract native input bindings. std::vector in_bind_vec; for (MySqlBindingPtr in_binding : in_bindings) { @@ -359,7 +369,7 @@ public: int status = 0; if (!in_bind_vec.empty()) { // Bind parameters to the prepared statement. - status = mysql_stmt_bind_param(statements_[index], + status = mysql_stmt_bind_param(holderHandle.statements_[index], in_bind_vec.empty() ? 0 : &in_bind_vec[0]); checkError(status, index, "unable to bind parameters for select"); } @@ -370,20 +380,20 @@ public: out_bind_vec.push_back(out_binding->getMySqlBinding()); } if (!out_bind_vec.empty()) { - status = mysql_stmt_bind_result(statements_[index], &out_bind_vec[0]); + status = mysql_stmt_bind_result(holderHandle.statements_[index], &out_bind_vec[0]); checkError(status, index, "unable to bind result parameters for select"); } // Execute query. - status = mysql_stmt_execute(statements_[index]); + status = mysql_stmt_execute(holderHandle.statements_[index]); checkError(status, index, "unable to execute"); - status = mysql_stmt_store_result(statements_[index]); + status = mysql_stmt_store_result(holderHandle.statements_[index]); checkError(status, index, "unable to set up for storing all results"); // Fetch results. - MySqlFreeResult fetch_release(statements_[index]); - while ((status = mysql_stmt_fetch(statements_[index])) == + MySqlFreeResult fetch_release(holderHandle.statements_[index]); + while ((status = mysql_stmt_fetch(holderHandle.statements_[index])) == MLM_MYSQL_FETCH_SUCCESS) { try { // For each returned row call user function which should @@ -427,26 +437,28 @@ public: template void insertQuery(const StatementIndex& index, const MySqlBindingCollection& in_bindings) { + MySqlHolder& holderHandle = handle(); std::vector in_bind_vec; + for (MySqlBindingPtr in_binding : in_bindings) { in_bind_vec.push_back(in_binding->getMySqlBinding()); } // Bind the parameters to the statement - int status = mysql_stmt_bind_param(statements_[index], + int status = mysql_stmt_bind_param(holderHandle.statements_[index], in_bind_vec.empty() ? 0 : &in_bind_vec[0]); checkError(status, index, "unable to bind parameters"); // Execute the statement - status = mysql_stmt_execute(statements_[index]); + status = mysql_stmt_execute(holderHandle.statements_[index]); if (status != 0) { // Failure: check for the special case of duplicate entry. - if (mysql_errno(mysql_) == ER_DUP_ENTRY) { + if (mysql_errno(holderHandle) == ER_DUP_ENTRY) { isc_throw(DuplicateEntry, "Database duplicate entry error"); } // Failure: check for the special case of WHERE returning NULL. - if (mysql_errno(mysql_) == ER_BAD_NULL_ERROR) { + if (mysql_errno(holderHandle) == ER_BAD_NULL_ERROR) { isc_throw(NullKeyError, "Database bad NULL error"); } checkError(status, index, "unable to execute"); @@ -470,30 +482,32 @@ public: template uint64_t updateDeleteQuery(const StatementIndex& index, const MySqlBindingCollection& in_bindings) { + MySqlHolder& holderHandle = handle(); std::vector in_bind_vec; + for (MySqlBindingPtr in_binding : in_bindings) { in_bind_vec.push_back(in_binding->getMySqlBinding()); } // Bind the parameters to the statement - int status = mysql_stmt_bind_param(statements_[index], + int status = mysql_stmt_bind_param(holderHandle.statements_[index], in_bind_vec.empty() ? 0 : &in_bind_vec[0]); checkError(status, index, "unable to bind parameters"); // Execute the statement - status = mysql_stmt_execute(statements_[index]); + status = mysql_stmt_execute(holderHandle.statements_[index]); if (status != 0) { // Failure: check for the special case of duplicate entry. - if ((mysql_errno(mysql_) == ER_DUP_ENTRY) + if ((mysql_errno(holderHandle) == ER_DUP_ENTRY) #ifdef ER_FOREIGN_DUPLICATE_KEY - || (mysql_errno(mysql_) == ER_FOREIGN_DUPLICATE_KEY) + || (mysql_errno(holderHandle) == ER_FOREIGN_DUPLICATE_KEY) #endif #ifdef ER_FOREIGN_DUPLICATE_KEY_WITH_CHILD_INFO - || (mysql_errno(mysql_) == ER_FOREIGN_DUPLICATE_KEY_WITH_CHILD_INFO) + || (mysql_errno(holderHandle) == ER_FOREIGN_DUPLICATE_KEY_WITH_CHILD_INFO) #endif #ifdef ER_FOREIGN_DUPLICATE_KEY_WITHOUT_CHILD_INFO - || (mysql_errno(mysql_) == ER_FOREIGN_DUPLICATE_KEY_WITHOUT_CHILD_INFO) + || (mysql_errno(holderHandle) == ER_FOREIGN_DUPLICATE_KEY_WITHOUT_CHILD_INFO) #endif ) { isc_throw(DuplicateEntry, "Database duplicate entry error"); @@ -502,7 +516,7 @@ public: } // Let's return how many rows were affected. - return (static_cast(mysql_stmt_affected_rows(statements_[index]))); + return (static_cast(mysql_stmt_affected_rows(holderHandle.statements_[index]))); } @@ -552,11 +566,13 @@ public: /// /// @throw isc::db::DbOperationError An operation on the open database has /// failed. - template + template void checkError(const int status, const StatementIndex& index, const char* what) const { + MySqlHolder& holderHandle = handle(); + if (status != 0) { - switch(mysql_errno(mysql_)) { + switch(mysql_errno(holderHandle)) { // These are the ones we consider fatal. Remember this method is // used to check errors of API calls made subsequent to successfully // connecting. Errors occurring while attempting to connect are @@ -570,13 +586,13 @@ public: DB_LOG_ERROR(db::MYSQL_FATAL_ERROR) .arg(what) .arg(text_statements_[static_cast(index)]) - .arg(mysql_error(mysql_)) - .arg(mysql_errno(mysql_)); + .arg(mysql_error(holderHandle)) + .arg(mysql_errno(holderHandle)); // If there's no lost db callback or it returns false, // then we're not attempting to recover so we're done if (!invokeDbLostCallback()) { - exit (-1); + exit(-1); } // We still need to throw so caller can error out of the current @@ -588,32 +604,43 @@ public: isc_throw(db::DbOperationError, what << " for <" << text_statements_[static_cast(index)] << ">, reason: " - << mysql_error(mysql_) << " (error code " - << mysql_errno(mysql_) << ")"); + << mysql_error(holderHandle) << " (error code " + << mysql_errno(holderHandle) << ")"); } } } - /// @brief Prepared statements - /// - /// This field is public, because it is used heavily from MySqlConnection - /// and will be from MySqlHostDataSource. - std::vector statements_; - /// @brief Raw text of statements /// /// This field is public, because it is used heavily from MySqlConnection - /// and will be from MySqlHostDataSource. + /// and from MySqlHostDataSource. std::vector text_statements_; /// @brief MySQL connection handle /// /// This field is public, because it is used heavily from MySqlConnection - /// and will be from MySqlHostDataSource. - MySqlHolder mysql_; + /// and from MySqlHostDataSource. + MySqlHolder& handle() const { + auto result = handles_.resource(); + // thread_local std::shared_ptr result(std::make_shared()); + if (connected_) { + result->openDatabase(*(const_cast(this))); + } + if (prepared_) { + result->prepareStatements(*(const_cast(this))); + } + return *result; + } + +private: + bool connected_; ///< Flag to indicate openDatabase has been called + + bool prepared_; ///< Flag to indicate prepareStatements has been called + + mutable isc::dhcp::ThreadResourceMgr handles_; }; -}; // end of isc::db namespace -}; // end of isc namespace +} // namespace db +} // namespace isc -#endif // MYSQL_CONNECTION_H +#endif // MYSQL_CONNECTION_H diff --git a/src/lib/mysql/tests/mysql_connection_unittest.cc b/src/lib/mysql/tests/mysql_connection_unittest.cc index 7d0a04d674..85e620fa19 100644 --- a/src/lib/mysql/tests/mysql_connection_unittest.cc +++ b/src/lib/mysql/tests/mysql_connection_unittest.cc @@ -63,11 +63,6 @@ public: try { // Open new connection. conn_.openDatabase(); - my_bool result = mysql_autocommit(conn_.mysql_, 1); - if (result != 0) { - isc_throw(DbOperationError, "failed to set autocommit option " - "for test MySQL connection"); - } // Create mysql_connection_test table. createTestTable(); @@ -124,22 +119,24 @@ public: /// /// @param sql Query in the textual form. void runQuery(const std::string& sql) { - MYSQL_STMT *stmt = mysql_stmt_init(conn_.mysql_); + MySqlHolder& holderHandle = conn_.handle(); + + MYSQL_STMT *stmt = mysql_stmt_init(holderHandle); if (stmt == NULL) { isc_throw(DbOperationError, "unable to allocate MySQL prepared " - "statement structure, reason: " << mysql_error(conn_.mysql_)); + "statement structure, reason: " << mysql_error(holderHandle)); } int status = mysql_stmt_prepare(stmt, sql.c_str(), sql.length()); if (status != 0) { isc_throw(DbOperationError, "unable to prepare MySQL statement <" - << sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << sql << ">, reason: " << mysql_errno(holderHandle)); } // Execute the prepared statement. if (mysql_stmt_execute(stmt) != 0) { isc_throw(DbOperationError, "cannot execute MySQL query <" - << sql << ">, reason: " << mysql_errno(conn_.mysql_)); + << sql << ">, reason: " << mysql_errno(holderHandle)); } // Discard the statement and its resources -- 2.47.2