Abstract LeaseMgr design

[Stephen Morris]

(Reposted to bind10-dhcp list: response at end)

On 06/09/13 10:29, Tomek Mrugalski wrote:
> I'd like to ask for your opinions on the possible ways abstract
> LeaseMgr could be extended to support PD. Right now we are talking
> about only one IPv6 lease - for addresses. Ultimately, there will
> be 3 types: IA_NA (non-temporary addresses, we have it already),
> IA_TA (temporary addresses, we don't care about them for now),
> IA_PD (prefixes, we need to add them).
> 
> Currently we have the following methods in LeaseMgr class:
> 
> Lease6Ptr getLease6(addr) const; Lease6Collection getLease6(duid,
> iaid) const; Lease6Ptr getLease6(duid, iaid, subnet_id) const; void
> updateLease6(lease6); bool deleteLease(addr);
> 
> There are couple ways we could add support for many lease types:
> 
> 1. check that getLease6() returns only the correct type of lease
> and get rid of any lease types that we don't care about. Pros: -
> LeaseMgr interface stays the same - No need to update specific
> backends Cons: - Spaghetti code in every place where LeaseMgr is
> used - Performance hit as we extract leases from the database that
> we don't care about and then toss them. - Hard to maintain in the
> long run
> 
> 2. add extra parameter (LeaseType) to every getLease6() method. So
> e.g.
> 
> Lease6Ptr getLease6(duid, iaid, subnet_id) const; would become 
> Lease6Ptr getLease6(leaseType, duid, iaid, subnet_id) const;
> 
> Pros: - Clean and easy interface - Quick abstract LeaseMgr
> implementation Cons: - Requires update of all backends (each backed
> will have to check the lease type)
> 
> 3. add extra parameter (LeaseType) to every getLease6() method, do
> not modify actual backends but add a verification layer in
> abstract LeaseMgr. That code would be really a one if (leaseType ==
> expectedType) per each method.
> 
> Pros: - Clean and easy interface - Does not require to update any
> backends Cons: - Requires changing abstract LeaseMgr (but that work
> is done once, not 3 times: MySQL, memfile and eventually PGSQL)
> 
> What are your thoughts? Did I miss any other alternatives? I
> personally prefer 2, because of its relative simplicity. Option 3
> is also appealing, but requires more work and slight update of the
> abstract LeaseMgr API. It is still better than 1 (no spaghetti
> code).
> 
> Background info: In a sane setup there will never be a lease type 
> mismatch. There are 2 cases I can come up with that could allow
> it, however. The first one is a clueless sysadmin that modifies a
> database. We can't prevent it from happening, we should just handle
> this error gracefully. The other case is when a clueless sysadmin
> configures address and prefix pools to overlap. We can implement
> sanity checks against it, but we won't be able to do that anytime
> soon.
> 
> Tomek

My preference is also for (2), the extra parameter plus back-end
modification.

All the SQL-based backends have the ability to select subsets of the
stored data based on more or less any criteria.  We should use that
ability as much as we can and minimise the code we write.

Although the addition of selection to the memfile backend might
require more work (but probably no more than the effort required for
the other two options), addition of it to the SQL backends such as
MySQL is trivial - just extend the WHERE clause. We probably won't
even need to add an index on lease type; I would expect that the
number of records returned by the other criteria to be small, so the
overhead of consulting an additional index in the query would probably
be greater than the overhead of scanning the few records in the
returned set.

> The other case is when a clueless sysadmin configures
>> address and prefix pools to overlap. We can implement sanity
>> checks against it, but we won't be able to do that anytime soon.

This makes me think that in the long-term it would be nice to have a
utility that would analyze at the lease database, and report any
inconsistencies. (And possibly repair them?)  At the very least, it
would be a useful diagnostic tool.

Stephen