Re: [SSSD] [PATCH] Fix startup race condition causing backend restarts
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/10/2009 08:36 AM, Dmitri Pal wrote:
Martin Nagy wrote:
> On Mon, 10 Aug 2009 06:52:11 -0400, Stephen Gallagher
> <sgallagh(a)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(a)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.
We're not proposing to go through the code and change what is already
done. We're proposing that, going forward, we start doing it a new way.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
- --
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkqAGcoACgkQeiVVYja6o6PFNQCeNEO0sKFcWAnDEOYcKJ/idoxk
M3cAnjR3HPDPEVtZSMVPlav8xuefv3Vd
=Dk1G
-----END PGP SIGNATURE-----