[kea-dev] Call for comments about the Decline design

[Thomas Markwalder]

On 7/28/15 3:38 AM, Marcin Siodelski wrote:
>
> On 17.07.2015 17:01, Thomas Markwalder wrote:
>> On 7/13/15 10:54 AM, Tomek Mrugalski wrote:
>>> Hi everyone,
>>> As part of the Kea 1.0 preparation, I wrote a short document about our
>>> intended design for Decline support. It is described here:
>>>
>>> http://kea.isc.org/wiki/DeclineDesign
>>>
>>> The major idea is to use special hardware address or duid values to
>>> indicate a declined address and keep it in the regular lease database.
>>> With this approach, the amount of work is greatly reduced, there is
>>> almost no performance degradation and this approach is proven
>>> (implemented years ago in Dibbler) to work well.
>>>
>>> I'd like to hear your opinions on this proposal. We plan to conclude the
>>> design discussions around end of July.
>>>
>>> Tomek
>>>
>>> _______________________________________________
>>> kea-dev mailing list
>>> kea-dev at lists.isc.org
>>> https://lists.isc.org/mailman/listinfo/kea-dev
>> I am not crazy about special purposing HWADDR/DUID to indicate a that lease
>> has been declined. I get that it is "less expensive" in the short term
>> from a development standpoint. I understand the appeal of that rationale but
>> if that is the only motivation it isn't enough.
>>
>> From an analysis standpoint HWADDR/DUID are values that identify DHCP
>> clients not a indicator of lease state.  We all know that proper API design
>> frowns upon multi-meaning parameters and the reasons why. 
>>
>> We must also ask ourselves if there are any other circumstances under
>> which we might persist a lease other than when it has been granted or
>> declined?  If so, would we need to retain the actual client values for
>> HWADDR/DUID or would we define more special values?  If we think there
>> is a reasonable possibility of other cases where we would persist leases,
>> then we should design for that now.  A more extensible approach would be
>> to add a state or status column.
>>
>> I realize that would leave us with a question of what values would we then
>> store in these columns when a lease is declined.  Again, from an analysis
>> standpoint the most meaningful values would be those of the declining
>> client.
>> Suppose we did retain the value(s) from the declining client, what are the
>> ramifications there?  It is already conceivable that a returning client
>> could
>> find a previous but expired lease right? We must be checking for that
>> now right? 
>> Bear in mind too, that using the "extra" effort it takes to test a flag or
>> state column is no more than the effort the it takes to to test for a
>> special
>> value in HWADDR/DUID.
>>
>> Similarly, using the viewpoint of "don't change the schema" if we can
>> squeeze
>> it in somewhere else is short-sighted thinking.  Not just in this matter
>> but in
>> any other.  Eventually short-cutting the solution to "save time" ALWAYS
>> comes
>> back to haunt you.  It may be faster now but we will feel the pain
>> later.  We
>> have schema-upgrading now built into the project so we should not be afraid
>> to change it. We should strive to do it right the first time.
>>
>> My gut instinct is that lease should have a state or status column and we
>> should use it to indicate that a lease has been declined.
>> ----------------------------------------------------------
>>
>> Regarding declined address processing via expiration - Should we really call
>> both the decline_recycle and expiration_reclaim hooks?  Seems to me that it
>> should only call the decline_recycle hook. From a business logic
>> standpoint it
>> wasn't an active lease so it cannot "expire" and therefore cannot be
>> reclaimed from
>> expiration.  If we call both then hooks developers may have to implement
>> logic
>> in their callouts to distinguish between the two business cases.
>>
>> What about the skip flag? How/where does this fit in?
>>
>> ----------------------------------------------------------
>>
>> Regarding the optional command to get "all declined", there needs to be
>> some guard
>> against how many we will return, if the number of leases is large it
>> could be an issue
>>
>> ----------------------------------------------------------
>>
>>
> Given all the comments for the lease expiration design and the decline
> design, I would like to propose that we actually implement the
> additional column 'state' for the Kea 1.0.
>
> The lease expiration, as it is described  now, marks the lease as
> 'reclaimed' by simply removing it from the lease database.  This
> minimizes changes to the lease database backends etc. However, I am
> quite confident (after some discussions I had so far) that at some point
> we will want 'address affinity'. In that case, removing the lease on
> expiration is not a right approach.
>
> Since we need to add new functions and indexes to retrieve expired
> leases, *now* seems to be the best time to add the new 'state' field if
> we want to go down this path.
>
> The reason I bring it in this thread is that at this point we may also
> consider if this field can be used for declined addresses.
>
> The big question is how to encode the state of the lease. I'd be in
> favor of encoding a single state on an individual bit, in which case it
> would be possible to encode multiple states with a single INT value,
> i.e. the lease could be in multiple states simultaneously.
>
> I am seeking for an answer whether we want this or not before I further
> work on the lease expiration design, as almost all further updates are
> going to be based on this decision.
>
> Marcin
A bitmask as you suggest, is probably the most flexible.  And I believe
that the time to add the new field is now, assuming that is the consensus.

Thomas