Fix logging on unexpected errors and also log on timeout.

Signed-off-by: Hannes Eder <[email protected]>
---

Hi,

since the patch is already applied I send a delta to the last patch.

On Tue, Aug 4, 2009 at 11:51, Lars Ellenberg<[email protected]> wrote:
> On Wed, Jul 22, 2009 at 03:33:16PM +0200, Hannes Eder wrote:
>> This patch solves two problems:
>>
>>   - When external checks timeout frequently ldirectord runs out of
>>     file handles, because when alarm()-ing out of Perls system() an
>>     internal pipe is not reclaimed.  This is at least true for Perl
>>     5.8.8 on Linux, but likely to affect other versions of Perl and
>>     systems as well.  The pipe is used to send errno(3) from the
>>     child to the parent, in case internal exec(3) fails.
>>
>>   - Do not leave child behind on timeout, TERMinate it.
>>
>> How are they solved?
>>
>>   - Emulate Perls system() via Perls fork(), exec(), and waitpid() and
>>     by TERMinating the child on timeout.
>
> nice catch.
>
> But:
>
>> +sub system_timeout
>> +{
>
> ...
>> +             alarm 0;
>> +             if ($@) {
>> +                     # timeout
>> +                     if ($@ ne "timeout\n") {
>> +                             # propagate unexpected errors
>
> you don't propagate, you only log.  maybe you want to log the @arg
> along, so the error can be matched to the invocation?

You are right, comment fixed.  That what comes from copy/pasting from
the manpage.

> BUG (typo):
> this should be $@, not $!, right?
>> +                             &ld_log("system_timeout unexpected error: $!");
> ==>                             &ld_log("system_timeout unexpected error: 
> $@");

Thanks, you are right, as $@ does not contain the expected value of
"timeout\n" in that case.

> maybe you want to log "timed out" as well?
>                        else {
>                                &ld_log("system_timeout(@args) timed out, kill 
> -TERM child.");
>                        }
> (or something like that)

Done.

Cheers,
Hannes


diff -r c99a87f6075a ldirectord/ldirectord.in
--- a/ldirectord/ldirectord.in  Tue Aug 04 08:49:57 2009 +0200
+++ b/ldirectord/ldirectord.in  Tue Aug 04 13:00:30 2009 +0200
@@ -4457,8 +4457,13 @@
                if ($@) {
                        # timeout
                        if ($@ ne "timeout\n") {
-                               # propagate unexpected errors
-                               &ld_log("system_timeout unexpected error: $!");
+                               # log unexpected errors
+                               &ld_log("system_timeout($timeout, @args) " .
+                                       "unexpected error: $@");
+                       }
+                       else {
+                               &ld_log("system_timeout($timeout, @args) " .
+                                       "timed out, kill -TERM child");
                        }
 
                        # TERMinate child
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to