On Tue, 2012-11-27 at 11:02 +0100, Michal Židek wrote: > On 11/26/2012 05:43 PM, Simo Sorce wrote: > > On Thu, 2012-11-22 at 09:52 +0100, Michal Židek wrote: > >> On 11/21/2012 04:41 PM, Simo Sorce wrote: > >>> On Wed, 2012-11-21 at 15:56 +0100, Michal Židek wrote: > >>>> On 11/21/2012 02:31 PM, Simo Sorce wrote: > >>>>> On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote: > >>>>>> On 11/20/2012 02:22 PM, Michal Židek wrote: > >>>>>>> Patch 1: sss_cache refactor. See patch description for more details. > >>>>>>> Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel > >>>>> > >>>>> It looks a bit excessive to remove the whole cache when you change 1 > >>>>> user, would be probably better to fix this by a slightly more complex > >>>>> construct. > >>>>> > >>>>> After you delete the data from the ldb cache you could query sssd_nss > >>>>> for the user name. And make sure that sssd_nss also searches the mc > >>>>> cache and remove the entry if it is found there on user lookup failure > >>>>> (we can later on use this feature to implement negative caching at the > >>>>> mc layer). > >>>>> > >>>>> Simo. > >>>>> > >>>> > >>>> Hmm... we would still need to handle the situation when sssd_nss is not > >>>> running. I guess removing the entire mmap cache here is just fine. > >>> > >>> What's the point of adding or deleting users if sssd is not running ? > >>> If you want to simply wipe the cache we have a specific tool for that. > >>> > >>>> But, to the case when sssd_nss is running. Does sss_nss_make_request() > >>>> only ldb requests? Or first mc then ldb? > >>> > >>> Only ldb, you need to add a mc request as an additional check, if the > >>> ldb search tunrs out empty. > >>> BAsically in the place where we do the negative caching now you'd add a > >>> call to a helper function to check and invalidate the mc cache for the > >>> specific user as well. > >>> > >>>> I am looking at the client code > >>>> now and it looks like this function is called when mc lookup did not > >>>> succeeded... so my guess is, that it only requests for ldb data. If so, > >>>> then this would be the function to use here. > >>>> > >>>> It would be like: > >>>> ... in the tool's code ... > >>>> sss_nss_make_request(SSS_NSS_GETPWNAM, ...); > >>>> > >>>> ... somewhere inside sssd_nss code ... > >>>> if (no_results_in_ldb) { > >>>> invalidate rec in mc > >>>> } > >>> > >>> Exactly. > >>> > >>>> Since the invalidation takes place inside sssd_nss process, no new > >>>> synchronization between processes should be needed (the barriers are > >>>> already there). But this would only work if sss_nss_make_request > >>>> communicates with ldb only (which I think it does). > >>>> > >>>> Was this your suggestion? > >>> > >>> Yes, precisely. > >>> > >>> Simo. > >>> > >> > >> However, it might still be good to push the originally proposed patches > >> as a temporary solution (or at least the first of them because the > >> sss_cache code is more readable after the refactor). > > > > Yes please rebase and resubmit the first patch on its own, it is already > > acked. > > > > For the second one, it seem we need a general mechanism to clear > > individual expired entries from the mc, I am working with Jakub on that, > > so once it is ready we should be able to wire it up from sss_* utils as > > well. > > > > So defer second for now. > > > > Simo. > > > > Sending the first patch only. Rebased against current master and updated > commit message.
Ack Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel