From: Razvan Becheriu <razvan@isc.org>
Date: Fri, 14 Aug 2020 09:34:01 +0000 (+0300)
Subject: [#1380] call mysql_autocommit on openDatabase
X-Git-Tag: Kea-1.8.0~63
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c8bd5a4693ac53f137e544fc6920330834b527f4;p=thirdparty%2Fkea.git

[#1380] call mysql_autocommit on openDatabase
---

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
index ea05b591d6..bdbd24f8d0 100644
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
@@ -58,18 +58,6 @@ MySqlConfigBackendImpl(const DatabaseConnection::ParameterMap& parameters)
 
     // Open the database.
     conn_.openDatabase();
-
-    // 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() {
diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index 175c3b4ffa..196b93d417 100644
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -2634,18 +2634,6 @@ MySqlHostDataSourceImpl::createContext() const {
     // Open the database.
     ctx->conn_.openDatabase();
 
-    // 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(ctx->conn_.mysql_, 1);
-    if (result != 0) {
-        isc_throw(DbOperationError, mysql_error(ctx->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.
diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index d191639fbb..6d35896345 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -1805,16 +1805,6 @@ MySqlLeaseMgr::createContext() const {
     // Open the database.
     ctx->conn_.openDatabase();
 
-    // 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(ctx->conn_.mysql_, 1);
-    if (result != 0) {
-        isc_throw(DbOperationError, mysql_error(ctx->conn_.mysql_));
-    }
-
     // Prepare all statements likely to be used.
     ctx->conn_.prepareStatements(tagged_statements.begin(),
                                  tagged_statements.end());
diff --git a/src/lib/mysql/mysql_connection.cc b/src/lib/mysql/mysql_connection.cc
index c95dc0b5b4..85877f4077 100644
--- a/src/lib/mysql/mysql_connection.cc
+++ b/src/lib/mysql/mysql_connection.cc
@@ -210,6 +210,24 @@ MySqlConnection::openDatabase() {
     if (status != mysql_) {
         isc_throw(DbOpenError, mysql_error(mysql_));
     }
+
+    // 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(mysql_, 1);
+    if (result != 0) {
+        isc_throw(DbOperationError, mysql_error(ctx->conn_.mysql_));
+    }
+
+    // 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.
 }
 
 // Get schema version.
diff --git a/src/lib/mysql/tests/mysql_connection_unittest.cc b/src/lib/mysql/tests/mysql_connection_unittest.cc
index cbd73bba19..0a12b132a0 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();