[Freeipa-devel] [PATCH 92 WIP] Flush whole zone from cache during zone renaming/removal.

[Petr Spacek]

Hello,

attached patch is preliminary implementation of selective zone flush.


Implementation is not so straight-forward as I want to see. Before discussing 
the patch itself - can we consider per-zone caches? In that case, we can 
simply deallocate whole per-zone RBT and we are done.

Pros:
* Potentially better concurrency, simpler code, much less corner cases.

Cons:
* We have to look into Zone register before searching the cache.
* It can limit concurrency ... with many extra small zones? I'm not sure.

------------

Now we can dive into all the gory details of single-tree-cache implementation:

Function discard_zone_from_cache() contains a long comment about potential 
problems, please send me your opinions.

Functions dbg_* can be simply deleted after end of testing.


I encountered some questions about locking:
How I should lock these two locks properly?
	RWLOCK(&zr->rwlock, isc_rwlocktype_read);
	LOCK(&cache->mutex);

AFAIK without some more intelligent algorithm or locking protocol it can 
simply deadlock if two threads attempt to get both locks in different order.

For now I chosen isc_task_beginexclusive() way. Hopefully, zone flush should 
be rare operation so it can be enough.

It raises another question:
Is it possible for a thread to hold some lock during isc_task_beginexclusive()?

I mean this situation:
thread 1: lock(&cache->mutex)
thread 1: store a pointer to the middle of the cache
thread 2: isc_task_beginexclusive()
thread 2: do something with cache
thread 2: isc_task_endexclusive()
thread 1: dereference stored pointer -> CRASH - thread 2 changed the data and 
pointer is invalid ... but thread 1 held the lock!

I'm not really sure about this part of BIND. My guess:
During "exclusive mode" all threads except single one (= thread which called 
isc_task_beginexclusive()) are blocked somewhere near dispatch() ... so they 
do nothing and thus they should not hold any lock.

Is my guess correct? I looked into task.c and related code but I can't say "I 
understood!" :-(


Now the funny part - RBT tree before and after per-zone flush.

Expected behaviour when removing zone 'test.'
=============================================
// cache tree before "test." zone removal
. (empty node)
                     4.34.10.in-addr.arpa
                         89.4.34.10.in-addr.arpa
             brq.redhat.com
                 pspacek.brq.redhat.com
                 test.brq.redhat.com
     test
         e.test
             _kerberos.e.test
             _tcp.e.test (empty node)
                 _kerberos._tcp.e.test
                 _kerberos-master._tcp.e.test
                 _kpasswd._tcp.e.test
                 _ldap._tcp.e.test
             _udp.e.test (empty node)
                 _kerberos._udp.e.test
                 _kerberos-master._udp.e.test
                 _kpasswd._udp.e.test
                 _ntp._udp.e.test
             c182.e.test
             pspacek.e.test
             test.e.test
         rec.test
         sec.test
         sub.test
             ns.sub.test
             rec.sub.test


// cache tree after 'test.' zone removal
// zones 'e.test.', 'sub.test.' and 'sec.test.' are still present
// record 'rec.test.' disappeared
. (empty node)
                     4.34.10.in-addr.arpa
                         89.4.34.10.in-addr.arpa
             brq.redhat.com
                 pspacek.brq.redhat.com
                 test.brq.redhat.com
     test (empty node)
         e.test
             _kerberos.e.test
             _tcp.e.test (empty node)
                 _kerberos._tcp.e.test
                 _kerberos-master._tcp.e.test
                 _kpasswd._tcp.e.test
                 _ldap._tcp.e.test
             _udp.e.test (empty node)
                 _kerberos._udp.e.test
                 _kerberos-master._udp.e.test
                 _kpasswd._udp.e.test
                 _ntp._udp.e.test
             c182.e.test
             pspacek.e.test
             test.e.test
         sec.test
         sub.test
             ns.sub.test
             rec.sub.test

-- 
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bind-dyndb-ldap-pspacek-0092-WIP-Flush-whole-zone-from-cache-during-zone-renaming.patch
Type: text/x-patch
Size: 10490 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121115/58ba7bfb/attachment.bin>