[SSSD] [PATCHES] sss_cache tool invalidates records in memory cache

Michal Židek mzidek at redhat.com
Mon Sep 17 15:18:34 UTC 2012


On 09/17/2012 03:11 PM, Michal Židek wrote:
> On 09/14/2012 07:55 PM, Simo Sorce wrote:
>> On Fri, 2012-09-14 at 17:16 +0200, Michal Židek wrote:
>>> On 09/13/2012 07:59 PM, Simo Sorce wrote:
>>>> Main questions:
>>>> the invalidation functions seem to be called unconditionally, what
>>>> prevent cache invalidation durint a normal log rotation ?
>>>> Shouldn't cache invalidation happen exclusively when sss_cache is
>>>> asking
>>>> to drop caches ?
>>>>
>>>
>>> It would be nice to have these two operations separate, but I do not
>>> know how to do it if we are to use SIGHUP for this. The application
>>> does not know what is the origin of SIGHUP, so it has to do both
>>> operations. For now, I wrote FIXME comment to the code in the new patch.
>>
>> Before sending the SIGHUP touch a file in the same directory where the
>> cache files are, and in sssd_nss stat and check if that file is present.
>> If it is, remove it and clean the caches.
>>
>>> Question: What do we plan to use to signal SSSD to reload configuration
>>> during runtime? Isn't it SIGHUP too? I am not sure now, but if it is
>>> SIGHUP, then we need to reinitialize memory caches every time SIGHUP is
>>> received by monitor (this is what the patch does right know). So maybe
>>> it is not that bad after all.
>>
>> You really *do not* want purge the caches just because the configuration
>> was reloaded. That would be quite a waste.
>>
>>>>
>>>> Nack:
>>>>
>>>> I do not like the following function as it is largely a copy/paste of
>>>> sss_mmap_cache_init.
>>>> Why donp;t you simply pass a pointer to struct sss_mc_ctx *mc_ctx
>>>> release it, and then simply call sss_mmap_cache_init() from scratch ?
>>>>
>>>> Also n_elem is currently hardcoded, so saving it is not really useful,
>>>> however I would also *really* not save it as on cache invalidation you
>>>> will want to go and see if the configuration also changed and get the
>>>> new number of elements.
>>>> This is a perfect time to do that because you are killing the old file
>>>> and creating a new one so changing size during this operation is
>>>> actually a good idea (if needed).
>>>>
>>>> Same for memcache_timeout parameters (which are already parametrized
>>>> and
>>>> can be fetched from CONFDB_NSS_CONF_ENTRY / CONFDB_MEMCACHE_TIMEOUT
>>>>
>>>
>>> I removed the old invalidate function and created new called
>>> sss_mmap_cache_reinit. This function is used for re-initialize
>>> already initialized memory cache. I think the name is more suitable,
>>> because it now allows to change entry timeout and cache size.
>>>
>>> New patches are attached.
>>
>> Looks ok, but I do not like that you keep the n_elem variable.
>>
>> Please create a macro until we get a tunable option.
>>
>> Something like:
>> #define SSS_MC_CACHE_ELEMENTS 50000
>>
>> And just use that.
>>
>> Simo.
>>
>
>
> Ok. I changed it all a bit. The mechanism is now as follows:
> 1. sss_cache creates a file, that serves as flag with the meaning
> "It was me, who sended SIGHUP".
> 2. monitor receives sighup and checks if the flag (file) exist
>      - exists: signal (dbus signal) nss to clear memory cache
>      - not exists: rotate logs and  signal other services to rotate
>        logs too
>
> To achieve this the te_server_hup is no longer used by monitor, all
> what monitor uses to handle SIGHUP is monitor_hup. Other services
> still remain using te_server_hup.
>
> With the file flag, there is no need for nss_hup function. NSS will
> remain using the old te_server_hup function and new nss_clear_memcache
> function is introduced to handle the caches.
>
> The flag (file) is removed in the sss_mmap_cache_init function. The
> reason for not placing it to the monitor_hup handler was to avoid this
> scenario:
> 1. Someone by mistake calls sss_cache tool when sssd is not running
> 2. The flag (file) is created
> 3. after that, sssd is turned on
> 4. someone sends SIGHUP to monitor to rotate logs
> 5. monitor sees the flag (file)
> 6. sssd clears memory cache and does not rotate anything
> 7. someone thinks how good he cleared the caches (?!NOoo!?)

It should have been:
7. someone thinks how good he rotated logs

>
> When unlink is moved to init function, this will never happen (reinit
> calls init, so it should be OK).
>
> New patches are in attachment.
>
> Thanks
> Michal
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>




More information about the sssd-devel mailing list