From: Razvan Becheriu Date: Mon, 28 Oct 2019 17:27:43 +0000 (+0200) Subject: [#888,!573] implement pgsql thread handle X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5d41ab703734823670ddca0cc19a5437cd3f5f80;p=thirdparty%2Fkea.git [#888,!573] implement pgsql thread handle --- diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 7ea6f61e6b..633ea483e7 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -297,7 +297,7 @@ public: // most recently added host is different than the host id of the // currently processed host. if (hosts.empty() || row_host_id != hosts.back()->getHostId()) { - HostPtr host = retrieveHost(r, row, row_host_id); + HostPtr host(retrieveHost(r, row, row_host_id)); hosts.push_back(host); } } @@ -1263,7 +1263,7 @@ private: OptionPtr option_; }; -} // end of anonymous namespace +} // namespace namespace isc { namespace dhcp { @@ -1854,7 +1854,7 @@ TaggedStatementArray tagged_statements = { { // Using fixed scope_id = 3, which associates an option with host. {7, { OID_INT2, OID_BYTEA, OID_TEXT, - OID_VARCHAR, OID_BOOL, OID_TEXT, OID_INT8}, + OID_VARCHAR, OID_BOOL, OID_TEXT, OID_INT8 }, "insert_v4_host_option", "INSERT INTO dhcp4_options(code, value, formatted_value, space, " " persistent, user_context, host_id, scope_id) " @@ -1866,7 +1866,7 @@ TaggedStatementArray tagged_statements = { { // Using fixed scope_id = 3, which associates an option with host. {7, { OID_INT2, OID_BYTEA, OID_TEXT, - OID_VARCHAR, OID_BOOL, OID_TEXT, OID_INT8}, + OID_VARCHAR, OID_BOOL, OID_TEXT, OID_INT8 }, "insert_v6_host_option", "INSERT INTO dhcp6_options(code, value, formatted_value, space, " " persistent, user_context, host_id, scope_id) " @@ -1903,7 +1903,7 @@ TaggedStatementArray tagged_statements = { { } }; -}; // end anonymous namespace +} // namespace PgSqlHostDataSourceImpl:: PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters) @@ -1927,7 +1927,7 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters) isc_throw(DbOpenError, "PostgreSQL schema version mismatch: need version: " << code_version.first << "." << code_version.second - << " found version: " << db_version.first << "." + << " found version: " << db_version.first << "." << db_version.second); } @@ -1957,8 +1957,10 @@ uint64_t PgSqlHostDataSourceImpl::addStatement(StatementIndex stindex, PsqlBindArrayPtr& bind_array, const bool return_last_id) { + PgSqlHolder& holderHandle = conn_.handle(); uint64_t last_id = 0; - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, tagged_statements[stindex].nbparams, &bind_array->values_[0], &bind_array->lengths_[0], @@ -1987,7 +1989,9 @@ PgSqlHostDataSourceImpl::addStatement(StatementIndex stindex, bool PgSqlHostDataSourceImpl::delStatement(StatementIndex stindex, PsqlBindArrayPtr& bind_array) { - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + PgSqlHolder& holderHandle = conn_.handle(); + + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, tagged_statements[stindex].nbparams, &bind_array->values_[0], &bind_array->lengths_[0], @@ -2062,9 +2066,10 @@ PgSqlHostDataSourceImpl:: getHostCollection(StatementIndex stindex, PsqlBindArrayPtr bind_array, boost::shared_ptr exchange, ConstHostCollection& result, bool single) const { + PgSqlHolder& holderHandle = conn_.handle(); exchange->clear(); - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, tagged_statements[stindex].nbparams, &bind_array->values_[0], &bind_array->lengths_[0], @@ -2110,29 +2115,34 @@ 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); } -std::pair PgSqlHostDataSourceImpl::getVersion() const { +pair +PgSqlHostDataSourceImpl::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_HOST_DB_GET_VERSION); + + PgSqlHolder& holderHandle = conn_.handle(); const char* version_sql = "SELECT version, minor FROM schema_version;"; - PgSqlResult r(PQexec(conn_, version_sql)); + + PgSqlResult r(PQexec(holderHandle, version_sql)); if(PQresultStatus(r) != PGRES_TUPLES_OK) { isc_throw(DbOperationError, "unable to execute PostgreSQL statement <" - << version_sql << ">, reason: " << PQerrorMessage(conn_)); + << version_sql << ">, reason: " << PQerrorMessage(holderHandle)); } - uint32_t version; - PgSqlExchange::getColumnValue(r, 0, 0, version); + uint32_t major; + PgSqlExchange::getColumnValue(r, 0, 0, major); uint32_t minor; PgSqlExchange::getColumnValue(r, 0, 1, minor); - return (std::make_pair(version, minor)); + return (make_pair(major, minor)); } void @@ -2455,8 +2465,8 @@ ConstHostPtr PgSqlHostDataSource::get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const { if (!address.isV4()) { - isc_throw(BadValue, "PgSqlHostDataSource::get4(id, address) - " - " wrong address type, address supplied is an IPv6 address"); + isc_throw(BadValue, "PgSqlHostDataSource::get4(id, address): " + "wrong address type, address supplied is an IPv6 address"); } // Set up the WHERE clause value @@ -2475,8 +2485,9 @@ PgSqlHostDataSource::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); } @@ -2550,7 +2561,8 @@ PgSqlHostDataSource::get6(const SubnetID& subnet_id, // Miscellaneous database methods. -std::string PgSqlHostDataSource::getName() const { +std::string +PgSqlHostDataSource::getName() const { std::string name = ""; try { name = impl_->conn_.getParameter("name"); @@ -2560,7 +2572,8 @@ std::string PgSqlHostDataSource::getName() const { return (name); } -std::string PgSqlHostDataSource::getDescription() const { +std::string +PgSqlHostDataSource::getDescription() const { return (std::string("Host data source that stores host information" "in PostgreSQL database")); } @@ -2583,5 +2596,5 @@ PgSqlHostDataSource::rollback() { impl_->conn_.rollback(); } -}; // end of isc::dhcp namespace -}; // end of isc namespace +} // namespace dhcp +} // namespace isc diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 1ade373649..7180ea54fc 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -133,7 +133,7 @@ PgSqlTaggedStatement tagged_statements[] = { "SELECT address, hwaddr, client_id, " "valid_lifetime, extract(epoch from expire)::bigint, subnet_id, " "fqdn_fwd, fqdn_rev, hostname, " - "state, user_context " + "state, user_context " "FROM lease4 " "WHERE subnet_id = $1"}, @@ -307,6 +307,7 @@ PgSqlTaggedStatement tagged_statements[] = { "hwaddr = $13, hwtype = $14, hwaddr_source = $15, " "state = $16, user_context = $17 " "WHERE address = $18"}, + // ALL_LEASE4_STATS { 0, { OID_NONE }, "all_lease4_stats", @@ -333,7 +334,7 @@ PgSqlTaggedStatement tagged_statements[] = { { 0, { OID_NONE }, "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"}, // SUBNET_LEASE6_STATS { 1, { OID_INT8 }, @@ -341,7 +342,7 @@ PgSqlTaggedStatement tagged_statements[] = { "SELECT subnet_id, lease_type, state, leases as state_count" " FROM lease6_stat " " WHERE subnet_id = $1 " - " ORDER BY lease_type, state" }, + " ORDER BY lease_type, state"}, // SUBNET_RANGE_LEASE6_STATS { 2, { OID_INT8, OID_INT8 }, @@ -349,7 +350,8 @@ PgSqlTaggedStatement tagged_statements[] = { "SELECT subnet_id, lease_type, state, leases as state_count" " FROM lease6_stat " " WHERE subnet_id >= $1 and subnet_id <= $2 " - " ORDER BY subnet_id, lease_type, state" }, + " ORDER BY subnet_id, lease_type, state"}, + // End of list sentinel { 0, { 0 }, NULL, NULL} }; @@ -463,8 +465,7 @@ public: lease_ = lease; try { - addr_str_ = boost::lexical_cast - (lease->addr_.toUint32()); + addr_str_ = boost::lexical_cast(lease->addr_.toUint32()); bind_array.add(addr_str_); if (lease->hwaddr_ && !lease->hwaddr_->hwaddr_.empty()) { @@ -1018,10 +1019,11 @@ public: /// parameters (for all subnets), a subnet id for a single subnet, or /// a first and last subnet id for a subnet range. void start() { + PgSqlHolder& holderHandle = conn_.handle(); if (getSelectMode() == ALL_SUBNETS) { // Run the query with no where clause parameters. - result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, + result_set_.reset(new PgSqlResult(PQexecPrepared(holderHandle, statement_.name, 0, 0, 0, 0, 0))); } else { // Set up the WHERE clause values @@ -1039,7 +1041,7 @@ public: } // Run the query with where clause parameters. - result_set_.reset(new PgSqlResult(PQexecPrepared(conn_, statement_.name, + result_set_.reset(new PgSqlResult(PQexecPrepared(holderHandle, statement_.name, parms.size(), &parms.values_[0], &parms.lengths_[0], &parms.formats_[0], 0))); } @@ -1132,7 +1134,7 @@ PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters) // Now prepare the SQL statements. int i = 0; - for( ; tagged_statements[i].text != NULL ; ++i) { + for(; tagged_statements[i].text != NULL; ++i) { conn_.prepareStatement(tagged_statements[i]); } @@ -1158,7 +1160,9 @@ PgSqlLeaseMgr::getDBVersion() { bool PgSqlLeaseMgr::addLeaseCommon(StatementIndex stindex, PsqlBindArray& bind_array) { - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + PgSqlHolder& holderHandle = conn_.handle(); + + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, tagged_statements[stindex].nbparams, &bind_array.values_[0], &bind_array.lengths_[0], @@ -1206,8 +1210,10 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex, Exchange& exchange, LeaseCollection& result, bool single) const { + PgSqlHolder& holderHandle = conn_.handle(); const int n = tagged_statements[stindex].nbparams; - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, n, + + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, n, n > 0 ? &bind_array.values_[0] : NULL, n > 0 ? &bind_array.lengths_[0] : NULL, n > 0 ? &bind_array.formats_[0] : NULL, 0)); @@ -1273,8 +1279,7 @@ PgSqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr) const { PsqlBindArray bind_array; // LEASE ADDRESS - std::string addr_str = boost::lexical_cast - (addr.toUint32()); + std::string addr_str = boost::lexical_cast(addr.toUint32()); bind_array.add(addr_str); // Get the data @@ -1709,7 +1714,9 @@ PgSqlLeaseMgr::updateLeaseCommon(StatementIndex stindex, LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_ADD_ADDR4).arg(tagged_statements[stindex].name); - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + PgSqlHolder& holderHandle = conn_.handle(); + + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, tagged_statements[stindex].nbparams, &bind_array.values_[0], &bind_array.lengths_[0], @@ -1748,9 +1755,8 @@ PgSqlLeaseMgr::updateLease4(const Lease4Ptr& lease) { exchange4_->createBindForSend(lease, bind_array); // Set up the WHERE clause and append it to the SQL_BIND array - std::string addr4_ = boost::lexical_cast - (lease->addr_.toUint32()); - bind_array.add(addr4_); + std::string addr_str = boost::lexical_cast(lease->addr_.toUint32()); + bind_array.add(addr_str); // Drop to common update code updateLeaseCommon(stindex, bind_array, lease); @@ -1778,7 +1784,9 @@ PgSqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { uint64_t PgSqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex, PsqlBindArray& bind_array) { - PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + PgSqlHolder& holderHandle = conn_.handle(); + + PgSqlResult r(PQexecPrepared(holderHandle, tagged_statements[stindex].name, tagged_statements[stindex].nbparams, &bind_array.values_[0], &bind_array.lengths_[0], @@ -1929,25 +1937,22 @@ PgSqlLeaseMgr::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_GET_VERSION); + PgSqlHolder& holderHandle = conn_.handle(); const char* version_sql = "SELECT version, minor FROM schema_version;"; - PgSqlResult r(PQexec(conn_, version_sql)); + + PgSqlResult r(PQexec(holderHandle, version_sql)); if(PQresultStatus(r) != PGRES_TUPLES_OK) { isc_throw(DbOperationError, "unable to execute PostgreSQL statement <" - << version_sql << ", reason: " << PQerrorMessage(conn_)); + << version_sql << ">, reason: " << PQerrorMessage(holderHandle)); } - istringstream tmp; - uint32_t version; - tmp.str(PQgetvalue(r, 0, 0)); - tmp >> version; - tmp.str(""); - tmp.clear(); + uint32_t major; + PgSqlExchange::getColumnValue(r, 0, 0, major); uint32_t minor; - tmp.str(PQgetvalue(r, 0, 1)); - tmp >> minor; + PgSqlExchange::getColumnValue(r, 0, 1, minor); - return (make_pair(version, minor)); + return (make_pair(major, minor)); } void @@ -1960,5 +1965,5 @@ PgSqlLeaseMgr::rollback() { conn_.rollback(); } -}; // end of isc::dhcp namespace -}; // end of isc namespace +} // namespace dhcp +} // namespace isc diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc index e8e149d482..88238f0d88 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -119,9 +119,11 @@ public: PgSqlConnection conn(params); conn.openDatabase(); - PgSqlResult r(PQexec(conn, query.c_str())); + PgSqlHolder& holderHandle = conn.handle(); + + PgSqlResult r(PQexec(holderHandle, query.c_str())); if (PQresultStatus(r) != PGRES_TUPLES_OK) { - isc_throw(DbOperationError, "Query failed:" << PQerrorMessage(conn)); + isc_throw(DbOperationError, "Query failed:" << PQerrorMessage(holderHandle)); } int numrows = PQntuples(r); @@ -644,9 +646,14 @@ TEST_F(PgSqlHostDataSourceTest, testAddRollback) { PgSqlConnection conn(params); ASSERT_NO_THROW(conn.openDatabase()); - PgSqlResult r(PQexec(conn, "DROP TABLE IF EXISTS ipv6_reservations")); - ASSERT_TRUE (PQresultStatus(r) == PGRES_COMMAND_OK) - << " drop command failed :" << PQerrorMessage(conn); + PgSqlHolder& holderHandle = conn.handle(); + + ConstHostCollection collection = hdsptr_->getAll4(0); + ASSERT_EQ(collection.size(), 0); + + PgSqlResult r(PQexec(holderHandle, "DROP TABLE IF EXISTS ipv6_reservations")); + ASSERT_TRUE(PQresultStatus(r) == PGRES_COMMAND_OK) + << " drop command failed :" << PQerrorMessage(holderHandle); // Create a host with a reservation. HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8:1::1", diff --git a/src/lib/pgsql/pgsql_connection.cc b/src/lib/pgsql/pgsql_connection.cc index af98f098cb..b33a7334aa 100644 --- a/src/lib/pgsql/pgsql_connection.cc +++ b/src/lib/pgsql/pgsql_connection.cc @@ -37,6 +37,62 @@ const int PGSQL_DEFAULT_CONNECTION_TIMEOUT = 5; // seconds const char PgSqlConnection::DUPLICATE_KEY[] = ERRCODE_UNIQUE_VIOLATION; +void +PgSqlHolder::setConnection(PGconn* connection) { + clearPrepared(); + if (pgconn_ != NULL) { + PQfinish(pgconn_); + } + pgconn_ = connection; + connected_ = false; + prepared_ = false; +} + +void +PgSqlHolder::clearPrepared() { + if (pgconn_ != NULL) { + // Deallocate the prepared queries. + if (PQstatus(pgconn_) == CONNECTION_OK) { + PgSqlResult r(PQexec(pgconn_, "DEALLOCATE all")); + if(PQresultStatus(r) != PGRES_COMMAND_OK) { + // Highly unlikely but we'll log it and go on. + DB_LOG_ERROR(PGSQL_DEALLOC_ERROR) + .arg(PQerrorMessage(pgconn_)); + } + } + } +} + +void +PgSqlHolder::openDatabase(PgSqlConnection& connection) { + if (connected_) { + return; + } + connected_ = true; + prepared_ = true; + connection.openDatabase(); + prepared_ = false; +} + +void +PgSqlHolder::prepareStatements(PgSqlConnection& connection) { + if (prepared_) { + return; + } + clearPrepared(); + // Prepare all statements queries with all known fields datatype + for (auto it = connection.statements_.begin(); + it != connection.statements_.end(); ++it) { + PgSqlResult r(PQprepare(pgconn_, (*it)->name, (*it)->text, + (*it)->nbparams, (*it)->types)); + if (PQresultStatus(r) != PGRES_COMMAND_OK) { + isc_throw(DbOperationError, "unable to prepare PostgreSQL statement: " + << (*it)->text << ", reason: " << PQerrorMessage(pgconn_)); + } + } + prepared_ = true; +} + PgSqlResult::PgSqlResult(PGresult *result) : result_(result), rows_(0), cols_(0) { if (!result) { @@ -103,7 +159,10 @@ PgSqlTransaction::PgSqlTransaction(PgSqlConnection& conn) PgSqlTransaction::~PgSqlTransaction() { // If commit() wasn't explicitly called, rollback. if (!committed_) { - conn_.rollback(); + try { + conn_.rollback(); + } catch (...) { + } } } @@ -114,28 +173,14 @@ PgSqlTransaction::commit() { } PgSqlConnection::~PgSqlConnection() { - if (conn_) { - // Deallocate the prepared queries. - if (PQstatus(conn_) == CONNECTION_OK) { - PgSqlResult r(PQexec(conn_, "DEALLOCATE all")); - if(PQresultStatus(r) != PGRES_COMMAND_OK) { - // Highly unlikely but we'll log it and go on. - DB_LOG_ERROR(PGSQL_DEALLOC_ERROR) - .arg(PQerrorMessage(conn_)); - } - } - } + statements_.clear(); + handle().clear(); } void PgSqlConnection::prepareStatement(const PgSqlTaggedStatement& statement) { - // Prepare all statements queries with all known fields datatype - PgSqlResult r(PQprepare(conn_, statement.name, statement.text, - statement.nbparams, statement.types)); - if(PQresultStatus(r) != PGRES_COMMAND_OK) { - isc_throw(DbOperationError, "unable to prepare PostgreSQL statement: " - << statement.text << ", reason: " << PQerrorMessage(conn_)); - } + statements_.push_back(&statement); + prepared_ = true; } void @@ -276,7 +321,10 @@ PgSqlConnection::openDatabase() { } // We have a valid connection, so let's save it to our holder - conn_.setConnection(new_conn); + PgSqlHolder& holderHandle = handle(); + holderHandle.setConnection(new_conn); + holderHandle.connected_ = true; + connected_ = true; } bool @@ -296,6 +344,9 @@ PgSqlConnection::checkStatementError(const PgSqlResult& r, // error class. Note, there is a severity field, but it can be // misleadingly returned as fatal. However, a loss of connectivity // can lead to a NULL sqlstate with a status of PGRES_FATAL_ERROR. + + PgSqlHolder& holderHandle = handle(); + const char* sqlstate = PQresultErrorField(r, PG_DIAG_SQLSTATE); if ((sqlstate == NULL) || ((memcmp(sqlstate, "08", 2) == 0) || // Connection Exception @@ -305,7 +356,7 @@ PgSqlConnection::checkStatementError(const PgSqlResult& r, (memcmp(sqlstate, "58", 2) == 0))) { // System error DB_LOG_ERROR(PGSQL_FATAL_ERROR) .arg(statement.name) - .arg(PQerrorMessage(conn_)) + .arg(PQerrorMessage(holderHandle)) .arg(sqlstate ? sqlstate : ""); // If there's no lost db callback or it returns false, @@ -321,7 +372,7 @@ PgSqlConnection::checkStatementError(const PgSqlResult& r, } // Apparently it wasn't fatal, so we throw with a helpful message. - const char* error_message = PQerrorMessage(conn_); + const char* error_message = PQerrorMessage(holderHandle); isc_throw(DbOperationError, "Statement exec failed:" << " for: " << statement.name << ", status: " << s << "sqlstate:[ " << (sqlstate ? sqlstate : "") @@ -332,9 +383,12 @@ PgSqlConnection::checkStatementError(const PgSqlResult& r, void PgSqlConnection::startTransaction() { DB_LOG_DEBUG(DB_DBG_TRACE_DETAIL, PGSQL_START_TRANSACTION); - PgSqlResult r(PQexec(conn_, "START TRANSACTION")); + + PgSqlHolder& holderHandle = handle(); + + PgSqlResult r(PQexec(holderHandle, "START TRANSACTION")); if (PQresultStatus(r) != PGRES_COMMAND_OK) { - const char* error_message = PQerrorMessage(conn_); + const char* error_message = PQerrorMessage(holderHandle); isc_throw(DbOperationError, "unable to start transaction" << error_message); } @@ -343,9 +397,12 @@ PgSqlConnection::startTransaction() { void PgSqlConnection::commit() { DB_LOG_DEBUG(DB_DBG_TRACE_DETAIL, PGSQL_COMMIT); - PgSqlResult r(PQexec(conn_, "COMMIT")); + + PgSqlHolder& holderHandle = handle(); + + PgSqlResult r(PQexec(holderHandle, "COMMIT")); if (PQresultStatus(r) != PGRES_COMMAND_OK) { - const char* error_message = PQerrorMessage(conn_); + const char* error_message = PQerrorMessage(holderHandle); isc_throw(DbOperationError, "commit failed: " << error_message); } } @@ -353,12 +410,16 @@ PgSqlConnection::commit() { void PgSqlConnection::rollback() { DB_LOG_DEBUG(DB_DBG_TRACE_DETAIL, PGSQL_ROLLBACK); - PgSqlResult r(PQexec(conn_, "ROLLBACK")); + + PgSqlHolder& holderHandle = handle(); + + PgSqlResult r(PQexec(holderHandle, "ROLLBACK")); if (PQresultStatus(r) != PGRES_COMMAND_OK) { - const char* error_message = PQerrorMessage(conn_); + const char* error_message = PQerrorMessage(holderHandle); isc_throw(DbOperationError, "rollback failed: " << error_message); } } -}; // end of isc::db namespace -}; // end of isc namespace +} // namespace db +} // namespace isc + diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h index 339ec7eefc..40dd05df6f 100644 --- a/src/lib/pgsql/pgsql_connection.h +++ b/src/lib/pgsql/pgsql_connection.h @@ -7,6 +7,7 @@ #define PGSQL_CONNECTION_H #include +#include #include #include @@ -160,11 +161,13 @@ public: } private: - PGresult* result_; ///< Result set to be freed - int rows_; ///< Number of rows in the result set - int cols_; ///< Number of columns in the result set + PGresult* result_; ///< Result set to be freed + int rows_; ///< Number of rows in the result set + int cols_; ///< Number of columns in the result set }; +/// @brief Forward declaration to @ref PgSqlConnection. +class PgSqlConnection; /// @brief Postgresql connection handle Holder /// @@ -179,35 +182,34 @@ private: /// For this reason, the class is declared noncopyable. class PgSqlHolder : public boost::noncopyable { public: - /// @brief Constructor /// /// Sets the Postgresql API connector handle to NULL. /// - PgSqlHolder() : pgconn_(NULL) { + PgSqlHolder() : connected_(false), prepared_(false), pgconn_(NULL) { } /// @brief Destructor /// /// Frees up resources allocated by the connection. ~PgSqlHolder() { - if (pgconn_ != NULL) { - PQfinish(pgconn_); - } + clear(); } + void clear() { + setConnection(NULL); + } + + void clearPrepared(); + /// @brief Sets the connection to the value given /// /// @param connection - pointer to the Postgresql connection instance - void setConnection(PGconn* connection) { - if (pgconn_ != NULL) { - // Already set? Release the current connection first. - // Maybe this should be an error instead? - PQfinish(pgconn_); - } + void setConnection(PGconn* connection); - pgconn_ = connection; - } + void openDatabase(PgSqlConnection& connection); + + void prepareStatements(PgSqlConnection& connection); /// @brief Conversion Operator /// @@ -217,19 +219,13 @@ public: return (pgconn_); } - /// @brief Boolean Operator - /// - /// Allows testing the connection for emptiness: "if (holder)" - operator bool() const { - return (pgconn_); - } + bool connected_; ///< Flag to indicate openDatabase has been called private: - PGconn* pgconn_; ///< Postgresql connection -}; + bool prepared_; ///< Flag to indicate prepareStatements has been called -/// @brief Forward declaration to @ref PgSqlConnection. -class PgSqlConnection; + PGconn* pgconn_; ///< Postgresql connection +}; /// @brief RAII object representing a PostgreSQL transaction. /// @@ -304,8 +300,8 @@ public: /// @brief Constructor /// /// Initialize PgSqlConnection object with parameters needed for connection. - PgSqlConnection(const ParameterMap& parameters) - : DatabaseConnection(parameters) { + PgSqlConnection(const ParameterMap& parameters) : + DatabaseConnection(parameters), connected_(false), prepared_(false) { } /// @brief Destructor @@ -400,30 +396,37 @@ public: void checkStatementError(const PgSqlResult& r, PgSqlTaggedStatement& statement) const; - /// @brief PgSql connection handle + /// @brief Raw statements /// - /// This field is public, because it is used heavily from PgSqlLeaseMgr + /// This field is public, because it is used heavily from PgSqlConnection /// and from PgSqlHostDataSource. - PgSqlHolder conn_; + std::vector statements_; - /// @brief Conversion Operator + /// @brief PgSql connection handle /// - /// Allows the PgConnection object to be passed as the context argument to - /// PQxxxx functions. - operator PGconn*() const { - return (conn_); + /// This field is public, because it is used heavily from PgSqlLeaseMgr + /// and from PgSqlHostDataSource. + PgSqlHolder& 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; } - /// @brief Boolean Operator - /// - /// Allows testing the PgConnection for initialized connection - operator bool() const { - return (conn_); - } +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 // PGSQL_CONNECTION_H diff --git a/src/lib/pgsql/pgsql_exchange.h b/src/lib/pgsql/pgsql_exchange.h index 4aa82b2235..fd9518a00f 100644 --- a/src/lib/pgsql/pgsql_exchange.h +++ b/src/lib/pgsql/pgsql_exchange.h @@ -70,7 +70,6 @@ struct PsqlBindArray { /// @return Returns true if there are no entries in the array, false /// otherwise. bool empty() const { - return (values_.empty()); } @@ -393,7 +392,7 @@ public: protected: /// @brief Stores text labels for columns, currently only used for /// logging and errors. - std::vectorcolumns_; + std::vector columns_; }; }; // end of isc::db namespace diff --git a/src/lib/pgsql/tests/pgsql_exchange_unittest.cc b/src/lib/pgsql/tests/pgsql_exchange_unittest.cc index 173665ac94..a1f597c3d7 100644 --- a/src/lib/pgsql/tests/pgsql_exchange_unittest.cc +++ b/src/lib/pgsql/tests/pgsql_exchange_unittest.cc @@ -198,18 +198,22 @@ public: " varchar_col VARCHAR(255) " "); "; - PgSqlResult r(PQexec(*conn_, sql)); + PgSqlHolder& holderHandle = conn_->handle(); + + PgSqlResult r(PQexec(holderHandle, sql)); ASSERT_EQ(PQresultStatus(r), PGRES_COMMAND_OK) - << " create basics table failed: " << PQerrorMessage(*conn_); + << " create basics table failed: " << PQerrorMessage(holderHandle); } /// @brief Destroys the basics table /// Asserts if the destruction fails void destroySchema() { if (conn_) { - PgSqlResult r(PQexec(*conn_, "DROP TABLE IF EXISTS basics;")); + PgSqlHolder& holderHandle = conn_->handle(); + + PgSqlResult r(PQexec(holderHandle, "DROP TABLE IF EXISTS basics;")); ASSERT_EQ(PQresultStatus(r), PGRES_COMMAND_OK) - << " drop basics table failed: " << PQerrorMessage(*conn_); + << " drop basics table failed: " << PQerrorMessage(holderHandle); } } @@ -227,10 +231,12 @@ public: /// Asserts if the result set status does not equal the expected outcome. void runSql(PgSqlResultPtr& r, const std::string& sql, int exp_outcome, int lineno) { - r.reset(new PgSqlResult(PQexec(*conn_, sql.c_str()))); + PgSqlHolder& holderHandle = conn_->handle(); + + r.reset(new PgSqlResult(PQexec(holderHandle, sql.c_str()))); ASSERT_EQ(PQresultStatus(*r), exp_outcome) << " runSql at line: " << lineno << " failed, sql:[" << sql - << "]\n reason: " << PQerrorMessage(*conn_); + << "]\n reason: " << PQerrorMessage(holderHandle); } /// @brief Executes a SQL statement and tests for an expected outcome @@ -250,7 +256,9 @@ public: PgSqlTaggedStatement& statement, PsqlBindArrayPtr bind_array, int exp_outcome, int lineno) { - r.reset(new PgSqlResult(PQexecPrepared(*conn_, statement.name, + PgSqlHolder& holderHandle = conn_->handle(); + + r.reset(new PgSqlResult(PQexecPrepared(holderHandle, statement.name, statement.nbparams, &bind_array->values_[0], &bind_array->lengths_[0], @@ -258,7 +266,7 @@ public: ASSERT_EQ(PQresultStatus(*r), exp_outcome) << " runPreparedStatement at line: " << lineno << " statement name:[" << statement.name - << "]\n reason: " << PQerrorMessage(*conn_); + << "]\n reason: " << PQerrorMessage(holderHandle); } /// @brief Fetches all of the rows currently in the table