[SSSD] [PATCH] test_memory_cache: Wait short time after cache invalidation

Michal Židek mzidek at redhat.com
Wed Aug 12 12:35:11 UTC 2015


On 08/12/2015 06:21 AM, Lukas Slebodnik wrote:
> On (11/08/15 14:43), Michal Židek wrote:
>> On 08/11/2015 01:41 PM, Lukas Slebodnik wrote:
>>> On (10/08/15 13:32), Michal Židek wrote:
>>>> On 08/10/2015 12:26 PM, Lukas Slebodnik wrote:
>>>>> On (10/08/15 10:41), Lukas Slebodnik wrote:
>>>>>> On (07/08/15 21:19), Michal Židek wrote:
>>>>>>> On 08/07/2015 06:16 AM, Lukas Slebodnik wrote:
>>>>>>>>
>>>>>>>> +def wait_till_nss_responder_invalidate_cache():
>>>>>>>> +    # 1 second (200 * 0.005) should be enough time for nss responder
>>>>>>>> +    for _ in range(1, 200):
>>>>>>>> +        if os.path.isfile(config.MCACHE_PATH + "/clear_mc_flag"):
>>>>>>>> +            time.sleep(.005)
>>>>>>>> +        else:
>>>>>>>> +            return
>>>>>>>> +
>>>>>>>> +    assert False, "nss responder didn't invalidate memory cache within second"
>>>>>>>
>>>>>>> Grammar nazi nitpick :) : Missing "a" -> within a second
>>>>>>> But the nitpick is not relevant due to the following:
>>>>>>>
>>>>>>> I would give it at least 5 seconds for 2 reasons:
>>>>>>> a) if it ends sooner nothing happens, everything is fine
>>>>>>> b) if it ends later (CI machine under heavy load
>>>>>>>    makes even this possible) it breaks the test.
>>>>>>>
>>>>>> I realized it might be better to implement it directly in the utility
>>>>>> sss_cache. I didn't increased time to 5 seconds because
>>>>>> It would be a long time for users
>>>>>> and they might to decide to cancel it (ctrl-c).
>>>>>>
>>>>> and now with patches
>>>>>
>>>>> LS
>>>>>
>>>>
>>>> 227
>>>> 228         ret = wait_till_nss_responder_invalidate_cache();
>>>> 229         if (ret != EOK) {
>>>> 230             ERROR("The fast memory caches was...
>>>> 231                   "responder.\n");
>>>> 232         }
>>>> 233     }
>>>>
>>>> We should not ignore the error here. If the function
>>>> returns EAGAIN we should propagate EAGAIN to
>>>> sss_memcache_clear_all() caller and let him call
>>>> the sss_memcache_clear_all() again with limited
>>>> number of tries. My proposal is shortening the
>>>> max_wait to 500 000 with 10 retries).
>>>>
>>> It is a teoretical and very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> unlikely situation. It can happen even nowadays but nobody has complained yet.
>>> It does worth to complicate code.
>>>
>>> The only difference between curent versions of sssd and this patche is that
>>> user will be informed about this teoretical and very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very very very very very very very very very very very very
>>> very very very very very situation.
>>                           ^^^^
>> You are missing "unlikely" here.
>>
>>>
>>> I'm willing to change error message because could be printed to users in other
>>> situation. Feel free to propose better one.
>>>
>>> LS
>>
>> I'm willing to accept your patches if the attached patch
>> is accepted as well so that this unlikely to happen bug
>> will not occur.
>>
>> I do not think that the fact that some situation is unlikely
>> to happen is a reason to ignore it,
> It's not ignored. The error message is printed.
>
>>From 6608ddc5789e98cea284229a79cd615a142debd8 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek at redhat.com>
>> Date: Tue, 11 Aug 2015 13:43:02 +0200
>> Subject: [PATCH] sss_cache: Try to remove mc files if last attempt failed
>>
>> When sss_cache checks if nss responder is running
>> and the answer is yes, but nss stops right after
>> that, the memory cache files may be left undeleted.
>>
>> So we should try at least one more time to remove
>> the files.
>> ---
> NACK
>
> Attached is alternative solution for issues in git master.
>
> LS
>

Seriously? Is that your proposed solution?

I am giving up.

It is better to go with the patches you sent before
(not the one that removes the tests, that would be
very wrong) rather than trying to continue this
discussion.

If someone wants to review the small patch I
sent as separate patch feel free to do so. I
still think it should be fixed.

Anyway, I am sending the Lukas's patches
again so it is easier for Jakub to push.

They are both ACKed.

CI passed on all machines (I disabled the problematic
tests):
http://sssd-ci.duckdns.org/logs/job/21/36/summary.html

I also ran local CI couple of times and it passed
always.

Michal

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sss_cache-Wait-a-while-for-invalidation-of-mc-by-nss.patch
Type: text/x-patch
Size: 2470 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150812/3d2ad773/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-test_memory_cache-Fix-few-python-issues.patch
Type: text/x-patch
Size: 2244 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150812/3d2ad773/attachment-0001.bin>


More information about the sssd-devel mailing list