[SSSD] [PATCH] implement cleanup task

Stephen Gallagher sgallagh at redhat.com
Tue Nov 10 13:58:10 UTC 2009


On 11/09/2009 05:32 PM, Simo Sorce wrote:
> On Mon, 2009-11-09 at 15:57 -0500, Stephen Gallagher wrote:
>> On 11/09/2009 02:20 PM, Simo Sorce wrote:
>>  
>>> So here are 2 patches:
>>> 0001 add some functions I needed for the task to sysdb and reworks a few
>>> others
>>
>> Nack.
>> sysdb_delete_user_send() should check whether both name and UID are
>> specified and fail if they are. Right now, if they are both set, we are
>> silently only using the name. Similarly, if neither is set (name==NULL
>> and uid==0) we should fail.
>>
>> Same comments about sysdb_delete_group_send().
>>
>> Otherwise it looks fine.
> 
> Ok I added code to verify that the uid/gid match the name being
> searched.
> 
>>>
>>> 0002 the actual cleanup task files.
>>
>> Nack.
>>
>> Please update manpages for ldap_purge_cache timeout, as well as the
>> SSSDConfig API configuration files.
> 
> Like for other timeouts I'd like to keep this undocumented for now.
> I added the option for the API configuration file.
> 
>> I don't like using SDAP_CACHE_PURGE_TIMEOUT for both the interval
>> between purge attempts AND the timeout for the purge running. Please add
>> one second to the interval time. Otherwise we're going to start queuing
>> up transaction waits on subsequent timeout tries if for some reason the
>> remote server is being very slow or unresponsive to our requests. Over a
>> long enough period of time, this could be a genuine resource problem.
>> I'd much prefer to guarantee that only one such request can be running
>> at any given time.
> 
> No this can't happen, new runs are not schedule until the previous run
> is over.
> 
>> The general approach looks fine to me, though I see a lot of duplication
>> between the cleanup_users_send() and cleanup_groups_send() functions and
>> I can't help wondering if there's a way that these could be made into a
>> single set of functions.
> 
> I know but I prefer to do things right first and optimize later,
> although there is some duplication here I preferred to keep things
> simpler than trying to add switch/case or if/else along the code to
> address all the differences.
> 
>> Also, can you add a set of tests to the sysdb-tests to manually set some
>> cache expiration times to "1" and verify that the cleanup task clears
>> them appropriately. (For regression-detection)
> 
> I'll open a bug to add sysdb-tests for these cases.
> 
> Also while checking the enumeration/cleanup code about the tiemouts I
> find out there where 2 places where the wrong timeout being set. That
> would have caused busy loops, because we were telling the code to
> schedule the next task for "now" in case we were offline, oops :)
> 
> I also changed the cleanup send function to take a memory context so
> that when called from the enumeration task it will actually be a child
> of that task (therefore it will be killed if the enum task timeout
> kicks-in).
> 
> Simo.
> 
> 
> 
> 
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel

Patch 0001: Nack, causes segfault in sysdb-tests. Attaching backtrace.

Patch 0002: Ack.

-- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: bt.txt
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20091110/34c0a74b/attachment.txt>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20091110/34c0a74b/attachment.sig>


More information about the sssd-devel mailing list