Re: svn commit: r799334 - /httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c

[jean-frederic clere <jf...@gmail.com>]

On 07/30/2009 09:27 PM, Ruediger Pluem wrote:
>
> On 07/30/2009 05:37 PM, jfclere@apache.org wrote:
>> Author: jfclere
>> Date: Thu Jul 30 15:37:22 2009
>> New Revision: 799334
>>
>> URL: http://svn.apache.org/viewvc?rev=799334&view=rev
>> Log:
>> Add the file logic for the handler.
>> (Next step add the slotmem logic for the multicast socket).
>>
>> Modified:
>>      httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
>>
>> Modified: httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c?rev=799334&r1=799333&r2=799334&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c (original)
>> +++ httpd/httpd/trunk/modules/cluster/mod_heartmonitor.c Thu Jul 30 15:37:22 2009
>

+++ CUT +++

>> +
>> +                if (apr_table_get(hbt, "lastseen")) {
>> +                    node.seen = atoi(apr_table_get(hbt, "lastseen"));
>> +                }
>> +                seen = fage + node.seen;
>> +                apr_file_printf(fp, "%s&ready=%u&busy=%u&lastseen=%u\n",
>> +                                ip, node.ready, node.busy, (unsigned int) seen);
>
> The above seems to be a lot of parsing back and forth for just updating the lastseen parameter.
> Couldn't we just keep the complete line until the 'lastseen=' and just update the number afterwards?

If only mod_heartmonitor writes there yes otherwise no.
Probably looking for lastseen= and digits should be enough.

> Or is the order of ready, busy and lastseen not defined?

If the order is defined we could use a structure but we would loose all 
the flexibility the actual format allows.

>
>
>> +    }
>> +
>> +    rv = apr_file_rename(path, ctx->storage_path, pool);
>
> We need to consider that multiple handlers run in parallel in different threads / processes.
> If we are in bad luck this can lock out node A as in the following example.
>
> Node A and Node B sent the request at the same time and the handlers processing these requests are
> running fairly in parallel.
> If the handler processing Node B does the renaming one tick later then the handler processing
> Node A then the update of Node A is lost. If this happens over and over again Node A is believed
> to be dead although it is not.
> I fear you need to use global locks here to avoid this.

Yes, I will work of that asap.

Cheers

Jean-Frederic