[Freeipa-devel] [PATCH 0042] Flush zones and RRs cache when handling persistent search reconnection

[Petr Spacek]

On 08/16/2012 02:51 PM, Adam Tkac wrote:
> On Wed, Aug 15, 2012 at 03:55:01PM +0200, Petr Spacek wrote:
>> On 08/15/2012 03:11 PM, Adam Tkac wrote:
>>> On Fri, Jul 27, 2012 at 12:16:07PM +0200, Petr Spacek wrote:
>>>> Hello,
>>>>
>>>> this patch implements "Flush zones and RRs cache when handling
>>>> persistent search reconnection" behaviour as requested
>>>> in ticket https://fedorahosted.org/bind-dyndb-ldap/ticket/44 .
>>>>
>>>> Petr^2 Spacek
>>>
>>>> +isc_result_t
>>>> +flush_ldap_cache(ldap_cache_t *cache)
>>>> +{
>>>> +	isc_result_t result;
>>>> +
>>>> +	REQUIRE(cache != NULL);
>>>> +
>>>> +	LOCK(&cache->mutex);
>>>> +	if (!ldap_cache_enabled(cache)) {
>>>> +		result = ISC_R_SUCCESS;
>>>> +	} else {
>>>> +		dns_rbt_destroy(&cache->rbt);
>>>> +		CHECK(dns_rbt_create(cache->mctx, cache_node_deleter, NULL,
>>>> +				&cache->rbt));
>>>
>>> In my opinion usage of dns_rbt_deletename(cache->rbt, dns_rootname, ISC_TRUE) is
>>> better, isn't it?
>>
>> I looked into implementation of both functions. dns_rbt_deletenode()
>> does horribly complicated magic for simple task as "delete whole
>> tree" is.
>>
>> For this reason I called dns_rbt_destroy() - it is much simpler and
>> should be faster (it doesn't try to maintain RBT invariants during
>> whole process).
>
> Sounds fine, ack for the patch as is.
>
> A

Pushed to master:
https://fedorahosted.org/bind-dyndb-ldap/changeset/468329216825f1694e0163f12c9f6d7c50dcc075

It closes the ticket:
https://fedorahosted.org/bind-dyndb-ldap/ticket/44

Further improvement will be handled as ticket:
https://fedorahosted.org/bind-dyndb-ldap/ticket/86

Petr^2 Spacek

>
>>>
>>> Otherwise OK.
>>>
>>>> +	}
>>>> +
>>>> +cleanup:
>>>> +	if (result != ISC_R_SUCCESS)
>>>> +		log_error_r("cache flush failed");
>>>> +	UNLOCK(&cache->mutex);
>>>> +	return result;
>>>> +}
>>>
>>> Regards, Adam
>>>
>>
>