Opened 7 years ago

Closed 4 years ago

#3164 closed defect (fixed)

MySQL connection timeout for invalid hosts is can be large

Reported by: tmark Owned by: stephen
Priority: low Milestone: Kea1.1
Component: database-all Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: .5
Total Hours: 7.5 Internal?: no

Description (last modified by tomek)

Dhcpsrc unit test, MySqlOpenTest?.OpenDatabase?, was taking two minutes to complete on a fresh install of Fedora 19 with the community version of MySQL. This exposes a minor issue with the connection timeout value not defaulting to reaonsably small values. Since the server is presumed to be local to the DHCP servers, setting the connection timeout to a reasonable if not configurable value would be prudent. The following patch would
set the value at 5 seconds:

diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index 9828085..12d5c39 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -1106,6 +1106,13 @@ MySqlLeaseMgr::openDatabase() {
                   mysql_error(mysql_));
     }
 
+    int connect_timeout = 5;
+    result = mysql_options(mysql_, MYSQL_OPT_CONNECT_TIMEOUT, &connect_timeout);
+    if (result != 0) {
+        isc_throw(DbOpenError, "unable to set connect timeout option: " <<
+                  mysql_error(mysql_));
+    }
+
     // Set SQL mode options for the connection:  SQL mode governs how what 
     // constitutes insertable data for a given column, and how to handle
     // invalid data.  We want to ensure we get the strictest behavior and

Subtickets

Change History (13)

comment:1 Changed 6 years ago by tomek

  • Milestone changed from DHCP Outstanding Tasks to DHCP-Kea1.0-alpha

comment:2 Changed 6 years ago by stephen

  • Milestone changed from DHCP-Kea0.9-alpha to DHCP-Kea0.9-beta

comment:3 Changed 6 years ago by tomek

  • Milestone changed from DHCP-Kea0.9 to DHCP Outstanding Tasks

comment:4 Changed 4 years ago by tomek

  • Description modified (diff)
  • Milestone changed from DHCP Outstanding Tasks to Kea1.1
  • Version set to git

comment:5 Changed 4 years ago by stephen

  • Owner set to stephen
  • Status changed from new to accepted

comment:6 Changed 4 years ago by stephen

  • Owner changed from stephen to UnAssigned
  • Status changed from accepted to reviewing

Ready for review.

I set a default connection timeout of 5 seconds for both MySQL and PostgreSQL. On the assumption that someone, somewhere would want to change it, I have made it a configurable parameter.

comment:7 Changed 4 years ago by stephen

  • Total Hours changed from 0 to 6

comment:8 Changed 4 years ago by tmark

  • Owner changed from UnAssigned to tmark

comment:9 Changed 4 years ago by tmark

  • Add Hours to Ticket changed from 0 to 1
  • Total Hours changed from 6 to 7

I updated the copyright date in admin guide so please pull this. A couple of minor things:

Technically, the parameter for MYSQL_OPT_CONNECT_TIMEOUT is an unsigned int* and you are fetching the config value into an int. Arguably you should declare connect_timeout as an unsigned int and then use boost::lexical_cast<unsigned int>. If you change this one you should probably change Postgresql code also.

In the admin documentation for the connection-timeout parameter, you may want to add a bit about the value having to be greater than zero. Timeouts of zero sometimes have a specific implication like "forever" or "immediate". In this case, it isn't valid.

Also, we'll need a ChangeLog? entry for this.

Beyond that, well done! ;)

comment:10 Changed 4 years ago by tmark

  • Owner changed from tmark to stephen

comment:11 Changed 4 years ago by stephen

  • Owner changed from stephen to tmark

Technically, the parameter for MYSQL_OPT_CONNECT_TIMEOUT is an unsigned int*

Well spotted. This has been changed from both MySQL and PostgreSQL.

In the admin documentation for the connection-timeout parameter, you may want to add a bit about the value having to be greater than zero.

Changed.

As to a ChangeLog entry, how about:

Make the database connection timeout a configurable parameter with a 
default value of 5 seconds.

comment:12 Changed 4 years ago by tmark

  • Add Hours to Ticket changed from 1 to .5
  • Owner changed from tmark to stephen
  • Total Hours changed from 7 to 7.5

Changes look fine as does the ChangeLog? entry. Please merge your changes.

comment:13 Changed 4 years ago by stephen

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged in commit 3332ad17523c6fcc1e735e4297169ebb2de95118

Note: See TracTickets for help on using tickets.