BIND 10 #2342: MySQL IPv6 lease database access functions

[BIND 10 Development]

#2342: MySQL IPv6 lease database access functions
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  tomek
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:  Sprint-
                   Priority:         |  DHCP-20121101
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  dhcpdb                             |           Sub-Project:  DHCP
                   Keywords:         |  Estimated Difficulty:  0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by tomek):

 Replying to [comment:10 stephen]:
 > > Also, do we need all those 11 headers if most of the code was removed?
 > What headers?
 There are 11 includes in lease_mgr.cc. I seriously doubt we need them all,
 especially that we are not doing anything fancy there.

 > '''mysql_lease_mgr.h'''
 > > Please do not use two leading underscores in MYSQL_LEASE_MGR_H define.
 > See above regarding what appears to be the norm in BIND 10.
 "what appears to be the norm" in a project cannot overrule standard
 restrictions.

 > > !StatementIndex. I think the GET_VERSION should come on the list
 first, as this is the first call to be ever called...
 > I was going to put the list of statements in alphabetical order.
 I was overthinking this. For a moment I got the (presumably false)
 impression that this will be an enum exported in a library and that you
 could say send query X. If that were true,  the first query you ever call
 having value of 1 would be beneficial. Since we are not likely to do it,
 you can safely ignore my comment. Those enum values will change between
 versions when we add new calls, but this will not be a problem.

 > > MySqlLease6Exchange fields: Why the duid storage is called clientid_
 and clientid_length_ rather than duid_ and duid_length_?
 > IIRC that was what is was called in the performance test program.  It
 has been changed to "duid".
 Ah yes. Dhcp-ubench was a strange mix. I tried to mimic additional load
 imposed by DHCPv6 data, while keeping the simplicity of 4 byte address.

 > It uses local time and comments have been added to note this.
 Ok, great. ISC-DHCP users will like that.

 > > MySqlLeaseMgr::getName() - should return "mysql";
 > As "name" is used to denote the database, getName() should return
 "keatest", which is how it has been implemented. (A "getType()" should
 return "mysql".)  We may want to check
 Ok. We can stick with that, but my original impression was that getName()
 is really getBackendName(). See existing comemnts in LeaseMgr::getName().

 > > One final question about the mysql_lease_mgr in general: is the code
 thread-safe?
 > Not at the moment.  The underlying mysql_xxxx() calls can be thread-
 safe, but you need to use a different database connection for each thread.
 If we go this way, we may want to relax the idea of a singleton lease
 manager - we could have multiple lease managers, each with a different
 connection.
 Ok. Let's keep it as they are now. I think the simplest way to go for now
 would be multi-process. We could start several server processes at the
 same time and each would create its own singleton and do allocations. I
 wrote allocation engine with such a model in mind. But let's not spend
 time on it. Keep things as they are now.

 > > One nasty question asked during BIND10 is also adequate here: is the
 code signal safe? I don't think we need to care about it now, just asking
 out of curiosity.
 > Probably not very signal safe.  There are no handlers I am aware of.
 It's not only a matter or handlers. Some functions may fail if they are
 interrupted by a call (even when signal handler is implemented).

-- 
Ticket URL: <http://bind10.isc.org/ticket/2342#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development