BIND 10 #356: Resolver address database

[BIND 10 Development]

#356: Resolver address database
-------------------------------+--------------------------------------------
      Reporter:  stephen       |        Owner:  stephen  
          Type:  enhancement   |       Status:  reviewing
      Priority:  major         |    Milestone:           
     Component:  Unclassified  |   Resolution:           
      Keywords:                |    Sensitive:  0        
Estimatedhours:  0.0           |        Hours:  0        
      Billable:  1             |   Totalhours:  0        
      Internal:  0             |  
-------------------------------+--------------------------------------------

Comment(by stephen):

 > The hash tables has mutex on each cell, to minimise the locking
 contention. However,
 > the LRU has only one global mutex and each lookup in the hash table will
 result in
 > one update of the LRU as well, so it seems the many mutexes on the hash
 tables are
 > useless, as the threads will collide in the LRU. Or is there a plan to
 address this
 > somehow?

 Good point, you are right, that was something I missed as the LRU list was
 added after I'd noted down my first thoughts about the hash table.

 One way round this is not to update the LRU on each lookup, but on each
 n'th lookup.  As entries are (or will be) protected by their own mutex,
 when modifying/accessing the entry, decrement a counter inside it.  When
 the counter reaches zero, reset it and move the entry to the tail of the
 LRU list.  Essentially instead of a LRU list it becomes a least-frequently
 used list.  By tweaking "n", we should be able to reduce contention for
 the global LRU lock.

 > I'm little bit concerned about locking of the data (zone entry and
 nameserver entry). > Currently it seems there's not enough of it. For
 example,
 > NameserverEntry::getExpiration does not lock, but the expiration_ may
 change, when
 > new referral data come.

 A call to getExpiration() that comes just before the expiration time were
 modified could (if there are thread scheduling delays) arrive just after.
 In that case, there is a small amount of randomness as to the value
 returned close to the time the value is reset anyway.  So it was felt that
 if the get were atomic, a lock would not add very much here.

 > With lru_data, size, getMaxSize and setMaxSize assume that
 > uint32_t are atomic types, which is not the case (or, is not guaranteed
 to be the
 > case in all architectures).

 You're right, there is a supposition that a 32-bit get or set is atomic.
 I'm assuming that BIND10 won't be available for 16-bit architectures
 (purely on the grounds of address space size). I'm also implicitly
 assuming that the compiler will align 32-values on 32-bit boundaries to
 allow atomic reads/writes elsewhere (although now that you've pointed it
 out, I must admit that I'm not sure that is the case in some of the modern
 64-bit architectures). But it is trivial to add a mutex specifically to
 protect those elements if needed.

 > Speaking about the sizes, why we use uint32_t? Shouldn't we use size_t?

 With uint32_t we know what the maximum value is and that it will be
 sufficient for our needs.  But I'm easy - we could use size_t if it is
 felt to be better.

 > Why we use string as the zone name and uint16_t as the class type?
 Shouldn't we use
 > isc::dns::Name and isc::dns::RRClass?

 The string as the zone name is a place holder for now; mainly because I
 was not sure what is the best format and what operations are needed.  I'm
 also wary about using Name if we need to construct intermediate objects -
 there is a lot of processing in the constructor which we might not need.

 As to class, you are right - it could be RRclass - although that is just a
 fancy wrapper around a uint16_t.

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