On 01/19/2016 11:56 AM, Pavel Březina wrote:
> On 01/19/2016 02:08 AM, Michal Židek wrote:
>> Hello,
>>
>> this patch applies on top of the
>> patch from thred:
>> [SSSD] [PATCH] NSS: Refresh also netgroup cache if needed
>>
>> It (re-)fixes the ticket:
>>
https://fedorahosted.org/sssd/ticket/2102
>>
>> I separated them to give this one special attention :)
>> and also because I am not sure if it is a good solution or
>> not.
>>
>> I would like pbrezina or preichl to review
>> this one, because they were the original
>> reviewer/author for the fix, so maybe they
>> still remember why it was done that way.
>>
>> Michal
>
> Nack.
>
>> if (ret == EOK) {
>> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid,
>> returning..\n");
>> return EOK;
>> } else if (ret == EAGAIN &&
>> dctx->domain->refresh_expired_interval
>> && req_type == SSS_DP_NETGR) {
>> /* Skip midpoint refresh if background refresh is enabled
>> * (for netgroups only)
>> */
>> return EOK;
>> } else if (ret != EAGAIN && ret != ENOENT) {
>> DEBUG(SSSDBG_CRIT_FAILURE, "Error checking cache: %d\n",
>> ret);
>> goto error;
>> }
>
> Please, return is_refresh_on_bg function and extend it to return true
> also for users and groups and merge the condition with the first one so
> debug message is visible, like so:
>
> if (ret == EOK || (ret == EAGAIN && is_refreshed_on_bg(...)) {
> DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid, returning..\n");
> return EOK;
> }
>
> Maybe you can merge those two patches together..
I wanted this one only in master. Do you think it should be
backported as well? I wanted only necessary changes in downstream.
Michal
I think it should be backported since "Tests showed that having midpoint
refresh enabled may actually slow down netgroup request occasionally." See: