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

Michal Židek mzidek at redhat.com
Mon Sep 17 13:11:05 UTC 2012


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!?)

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-tools_util.h-provides-signal_sssd-function.patch
Type: text/x-patch
Size: 7395 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120917/797add63/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sss_cache-tool-invalidates-records-in-memory-cache.patch
Type: text/x-patch
Size: 12592 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120917/797add63/attachment-0001.bin>


More information about the sssd-devel mailing list