Skip Menu |

This queue is for tickets about the GearmanX-Starter CPAN distribution.

Report information
The Basics
Id: 80472
Status: resolved
Priority: 0/
Queue: GearmanX-Starter

People
Owner: Nobody in particular
Requestors: DOUGW [...] cpan.org
Cc: sebastien.bocahu [...] nuxit.com
AdminCc:

Bug Information
Severity: Wishlist
Broken in: (no value)
Fixed in: 0.05



CC: sebastien.bocahu [...] nuxit.com
Subject: Option to sleep and retry on error
Add option to sleep and retry in the worker loop instead of dying.
Subject: GearmanXStarter.patch
--- Starter.pm.orig 2012-10-15 15:50:48.000000000 +0200 +++ Starter.pm 2012-10-15 16:05:03.000000000 +0200 @@ -35,6 +35,8 @@ my $sigterm = $args->{sigterm} || [ 'TERM' ]; + my $sleep = $args->{sleep_and_retry}; + $logger->info("Forking daemon for $worker_name") if $logger; _Init() and return 1; @@ -92,8 +94,14 @@ 1; }; if ( !$res && $@ !~ /GearmanXQuitLoop/ ) { - $logger->logdie("Error running loop for worker $worker_name [$@]:". $WORKER->error) - if $logger; + if ($sleep) { + $logger->logwarn("Error running loop for worker $worker_name [$@]:".$WORKER->error. + " -- Going to sleep for ${sleep}s") if $logger; + sleep $sleep; + } else { + $logger->logdie("Error running loop for worker $worker_name [$@]:".$WORKER->error) + if $logger; + } } last if $QUIT; }
On Mon Oct 29 13:12:23 2012, DOUGW wrote: Show quoted text
> Add option to sleep and retry in the worker loop instead of dying.
Posting here so I don't lose the patch. I'm not sure about this patch. I don't think I would want to sleep and retry on any error. Some errors should be fatal and not retried. This should probably be handled in the worker code as an eval which traps errors that you want to retry on and dies otherwise. But it probably depends on what the code is so maybe this is still an okay thing to do if you know that's what you want. I'll think about it.
Subject: Re: [rt.cpan.org #80472] Option to sleep and retry on error
Date: Tue, 30 Oct 2012 19:27:53 +0100
To: Douglas Wilson via RT <bug-GearmanX-Starter [...] rt.cpan.org>
From: Sébastien Bocahu <sebastien.bocahu [...] nuxit.com>
Show quoted text
> I'm not sure about this patch. I don't think I would want to sleep and > retry on any error. Some errors should be fatal and not retried. This > should probably be handled in the worker code as an eval which traps > errors that you want to retry on and dies otherwise. But it probably > depends on what the code is so maybe this is still an okay thing to do > if you know that's what you want. I'll think about it.
I agree that handling of the worker code errors should be done in the worker code. However, I can't see any good reason for the daemon to quit at any first gearman error. Maybe some errors are important enough for being considered as fatal, but a GEARMAN_TIMEOUT, for example, should be handled differently IMHO. And I can't see how such an error could be trapped from the worker code. It might be best to get a list of errors that should be considered as non-fatal, such as GEARMAN_LOST_CONNECTION, GEARMAN_TIMEOUT. thanks, -- Sébastien Bocahu
On Tue Oct 30 14:29:18 2012, sebastien.bocahu@nuxit.com wrote: Show quoted text
> > I agree that handling of the worker code errors should be done in the > worker > code. However, I can't see any good reason for the daemon to quit at > any first > gearman error.
That makes sense, but then the change should be near the check for GEARMAN_SUCCESS, and logwarn() there, not outside the eval loop. Show quoted text
> Maybe some errors are important enough for being > considered as > fatal, but a GEARMAN_TIMEOUT, for example, should be handled > differently IMHO.
Since I'm not familiar with the various GEARMAN errors, I'll not distinguish between them for now, but I'll take suggestions on which to handle as fatal...
On Tue Oct 30 20:06:11 2012, DOUGW wrote: Show quoted text
> On Tue Oct 30 14:29:18 2012, sebastien.bocahu@nuxit.com wrote: > > That makes sense, but then the change should be near the check for > GEARMAN_SUCCESS, and logwarn() there, not outside the eval loop.
New version uploaded. Please test, as I can't currently. Also if you want co-maintainer of this library, let me know, since I'm not even using gearman anymore.
On Tue Oct 30 20:23:16 2012, DOUGW wrote: Show quoted text
> On Tue Oct 30 20:06:11 2012, DOUGW wrote:
> > On Tue Oct 30 14:29:18 2012, sebastien.bocahu@nuxit.com wrote: > > > > That makes sense, but then the change should be near the check for > > GEARMAN_SUCCESS, and logwarn() there, not outside the eval loop.
> > New version uploaded. Please test, as I can't currently. Also if you > want co-maintainer of this library, let me know, since I'm not even > using gearman anymore.
I'm going to call this resolved.
Subject: Re: [rt.cpan.org #80472] Option to sleep and retry on error
Date: Mon, 12 Nov 2012 17:23:22 +0100
To: Douglas Wilson via RT <bug-GearmanX-Starter [...] rt.cpan.org>
From: Sébastien Bocahu <sebastien.bocahu [...] nuxit.com>
Show quoted text
> > > That makes sense, but then the change should be near the check for > > > GEARMAN_SUCCESS, and logwarn() there, not outside the eval loop.
> > > > New version uploaded. Please test, as I can't currently. Also if you > > want co-maintainer of this library, let me know, since I'm not even > > using gearman anymore.
> > I'm going to call this resolved.
Sorry, I've been very busy on other subjects. I just tested and it looks good to me. Thank you very much, -- Sébastien Bocahu Network & systems engineer Nuxit - http://nuxit.com