heder at google
Aug 4, 2009, 4:23 AM
Post #1 of 2
(516 views)
Permalink
|
|
[PATCH] Low: ldirectord: tweak system_timeout()
|
|
Fix logging on unexpected errors and also log on timeout.
Signed-off-by: Hannes Eder <heder [at] google>
---
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<lars.ellenberg [at] linbit> 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: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
|
