[kea-dev] Call for comments about the Lease Expiration design

[Thomas Markwalder]

On 7/27/15 8:22 AM, Marcin Siodelski wrote:
> Tom,
>
> Thanks for the review. My answers inline.
>
> On 14.07.2015 20:36, Thomas Markwalder wrote:
>> On 7/6/15 8:43 AM, Marcin Siodelski wrote:
>>> All,
>>>
>>> I have put together the document
>>> http://kea.isc.org/wiki/LeaseExpirationDesign, which presents the design
>>> for the Lease Expiration in Kea 1.0.
>>>
>>> This is the first pass for this document and it is still considered
>>> early draft. In particular there are two sections about extending the
>>> MySQL and PostgreSQL backend, which are marked "TBD". I am still
>>> wondering if I should put anything into these sections, given that the
>>> changes are straight forward and writing what we want to do may take
>>> more time and actually implementing it. Nonetheless, the document is in
>>> the shape in which people can review it and provide useful comments.
>>> Please to so!
>>>
>>> Marcin
>>> _______________________________________________
>>> kea-dev mailing list
>>> kea-dev at lists.isc.org
>>> https://lists.isc.org/mailman/listinfo/kea-dev
>> Nicely done.
>>
>> I find "most early" and "least early" confusing phrases.  The word
>> "early" means "before an expected time" or "in the beginning part of an
>> event".
>>
>> "most early"
>> By this you mean, the lease whose expiration date is further in the past
>> than any other lease?
>> If so, than it is the lease with the  "oldest expiration date" or the
>> "most expired". 
>>
>> "least early"
>> By this you mean, the lease whose expiration date is nearest to the
>> present than any other lease?
>> If so, than it is the lease with the  "newest expiration date" or the
>> "least expired".  So we
>> process them in order of oldest expiration to newest expiration, or most
>> expired to least expired.
>>
>> I would prefer oldest expiration/newest expiration or most expired/least
>> expired.
>>
> Ok. I don’t object to this change.
>
>> ------------------------------------------
>>
>> I think you should at least outline the MySQL/PostgreSQL changes.  If
>> there are schema changes, these have ramifications and these should be
>> identified.  They would impact db related tools and scripts,
>> and adding (or not adding) indexes have performance implications.
>>
> Yes. This was the goal. I didn’t do it because of the time defficiency
> (upcoming IETF meeting etc.). At the same time I wanted to initite the
> design discussion as soon as possible, to gather the feedback prior to
> the start of the 1.0 milestone.
> I’ll fill this gap in.
>
>> ------------------------------------------
>>
>> Leases Reclamation Routine -
>>
>> Where you list the operations to be performed by reclamation routines, you
>> might want to change the "bullets" to numerals and state that this is
>> the order
>> they will be performed.  This is relevant when discussing the hook points.
>>
> Ok. I’ll do it.
>
>> ------------------------------------------
>>
>> New Hook Points/Discussion and skip flag -
>>
>> Does it makes sense to honor the skip at all?  Unless they alter the lease
>> lifetime or cltt and update the lease in the db (or delete if from the
>> db) in their callout,
>> skipping reclamation just causes the lease to come up for reclamation on
>> the next cycle.
>> Other than just honoring it as a matter of ultimate flexibility, I'm not
>> sure I see a use-case,
>> unless of course they are implementing their own expiration mechanisms.
>>
>> If we do decide to honor the skip flag for reclamation hook point, then I
>> think it should only be done as all-or-nothing.  Either we reclaim it or we
>> skip it.  I don't think it makes much sense to let them delete it from
>> the DB
>> but skip updating DDNS.  We should also consider creating a statistic for
>> reclaim-skips so this can be tracked.    And yes, we would need to clearly
>> document the ramifications of skipping a reclamation.
>>
> The hooks framework is all about the flexibility of the user and users
> may break their own server heavily. But, we have this situation for
> other hook points too. I am in favor of letting users implement their
> own expiration mechanisms, if they want to. If they don’t want to do it,
> they just leave the „skip” flag alone.
>
> Here:
> "I don't think it makes much sense to let them delete it from the DB but
> skip updating DDNS”.
> Did you mean, „Skip updating DDNS but deleting the lease from the DB” ?
Correct.  If skip is false we update both the DB and DDNS, if it is
true, we do not update either one.
   
It is perhaps, unfortunate that the skipFlag is boolean, otherwise an
alternative would be to use it like a bitmask: SKIP_DB_UPDATE,
SKIP_DDNS_UPDATE,  SKIP_DB_UPDATE|SKIP_DDNS_UPDATE.     

>> ------------------------------------------
>>
>> I agree with your selection of Approach 2, implementing the TimeMgr, though
>> you don't really state who/what owns the worker thread in which the
>> IOService
>> is run.   TimeMgr itself might actually be the best location for this by
>> providing a method, runTimerThread(), that when invoked spawns off the
>> worker thread which simply invokes io_service_->run().  Similarly, it should
>> probably provide a stopTimeThread() method.   For that matter, TimeMgr, if
>> so desired could instantiate/use its own IOService instance.
>>
> I agree that the clarification is needed.
>
>> Although mentioned under approach 1, You cannot use io_service_->poll in
>> a loop,
>> as it returns immediately if no handlers are ready, so much of the time
>> it would
>> just spin like mad and go cpu-bound.
>>
> Duly noted.
>
>> ------------------------------------------
>>
>> Shouldn't there be an implementation task for retrofitting LFC
>> scheduling in the servers
>> to use TimerMgr?
>>
>>
>>
>>
> Yep. I’ll add one.
>
> Marcin