[SSSD] [PATCH] Fix startup race condition causing backend restarts

Dmitri Pal dpal at redhat.com
Mon Aug 10 12:36:39 UTC 2009


Martin Nagy wrote:
> On Mon, 10 Aug 2009 06:52:11 -0400, Stephen Gallagher
> <sgallagh at redhat.com> wrote:
>
>   
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 08/10/2009 03:38 AM, Martin Nagy wrote:
>>     
>>> On Fri, 07 Aug 2009 13:59:49 -0400, Stephen Gallagher
>>> <sgallagh at redhat.com> wrote:
>>>       
>>>> When the backends start up, the monitor was immediately sending a
>>>> getIdentity request. However, as we've added more processing to
>>>> the initialization routines over time, we started introducing
>>>> latency between when we open the connection and when we're able to
>>>> process requests on that connection.
>>>>
>>>> I've updated the monitor to check for NoReply as an error message
>>>> and queue the getIdentity check for retries until the service
>>>> answers or one wallclock second (literally, ten tries at 10ms
>>>> each) has passed.
>>>>         
>>> Hi Steven, the patch looks good to me. Ack.
>>> I do have some minor suggestions though, apply them at your own
>>> discretion.
>>>
>>>       
>>>>  static void identity_check(DBusPendingCall *pending, void *data)
>>>> @@ -1862,8 +1886,11 @@ static void identity_check(DBusPendingCall *pending, void *data)
>>>>      DBusError dbus_error;
>>>>      dbus_uint16_t svc_ver;
>>>>      char *svc_name;
>>>> +    const char *error_name;
>>>>      dbus_bool_t ret;
>>>>      int type;
>>>> +    struct timeval tv;
>>>> +    struct tevent_timer *te;
>>>>         
>>> I would put these two variables closer to where they are needed (marked
>>> below).
>>>
>>> [snip]
>>>
>>>       
>>>> +        error_name = dbus_message_get_error_name(reply);
>>>> +        if (strcmp(error_name, DBUS_ERROR_NO_REPLY) == 0) {
>>>>         
>>> Put the 'tv' and 'te' variable declarations here.
>>>
>>>       
>> I generally would, but the coding style within the SSSD prefers
>> declaration at the top of the function. If I have misunderstood our
>> coding style here, someone please tell me.
>>     
>
> You're right. I will send an email proposing to change this in the
> coding style.
>
>   
I am not against the change in general but I suggest we do not do it now.
This is a significant change to the way the code is structured or can be
structured.
Changing it now IMO is too late. It might make more harm than good.




More information about the sssd-devel mailing list