From: Ara.T.Howard@...
Date: 2004-08-25T02:25:41+09:00
Subject: Re: POLS - exception comparisons

On Tue, 24 Aug 2004, Robert Klemme wrote:

> I guess nooene ever bothered because exceptions are usually not stored
> somewhere.  You just throw them, catch them and print them.  But you usually
> don't put them in some kind of collection.  You could solve this by either
> using your comparison code (that compares stack traces etc.) or you convert
> the error to a string and compare those.
>
> Some additional remarks about your implementation:
>
> - I'd put the exception only into the array if it's the first one of this
> type.  Maybe you have good reasons to do it otherwise (to be able to later
> detect duplicates for example).

yes - uniq errors are warned on the way, iff we eventually succeed that's all
that happens.  however, if we eventually surpass the number of allowed errors
then ALL of them are logged at a fatal level (in order and including duplicates
for completeness) and the program then exits with a failing status.  the code i
posted is partial.

> - Recursion is inefficient to do what you want, rather use 'retry'.

ruby is inefficient.  this code is part of a clustering system that chugs
along running jobs that take between 30 minutes and 5 days so anything under
15 minutes is considered 'extremely effeicent' ;-)

> - Your method silently exits if it's a repeated error.  IMHO that's a bad
> solution since the error goes totally unnoticed (it's not logged and not
> raised either).

??

   ~ > ruby a.rb
   W, [2004-08-24T11:01:27.799685 #17735]  WARN -- : this is logged exactly once (RuntimeError)
   a.rb:19
   a.rb:19:in `times'
   a.rb:19
   a.rb:19:in `call'
   a.rb:4:in `error_wrap'
   a.rb:19
   a.rb:19
   a.rb:19: this is logged exactly once (RuntimeError)
           from a.rb:19:in `times'
           from a.rb:19
           from a.rb:19:in `call'
           from a.rb:4:in `error_wrap'
           from a.rb:12:in `error_wrap'
           from a.rb:11
           from a.rb:19


   ~ > cat a.rb
   require 'logger'
   def error_wrap errors = [], &block
     begin
       block.call errors
     rescue => e
       errors << e
       if errors.size >= @fault_tolerance
         raise
       else
         if errors.size == 1 or (errors.size >= 2 and errors[-2] != errors[-1])
           @logger.warn{ errors.last }
           error_wrap errors, &block
         end
       end
     end
   end
   @fault_tolerance = 2
   @logger = Logger.new STDERR
   error_wrap{ 2.times{ raise 'this is logged exactly once' } }

??

> - I don't like the idea that the block receives the errors array, but
> that's probably needed to be able to react on this.

it makes some routines be able to exit prematurely if certain very specific
conditions are met - but the general rule is to make @fault_tolerance attempts.

i use a general rule when i can't think of anything good to pass to a block -
if there is an execution context pass that, otherwise pass nothing.  in this
case the list of encountered errors is the context.


> So this would be my implementation of your method:
>
>   def error_wrap errors = [], &block
>     begin
>       block.call errors
>     rescue => e
>       if errors.size >= @fault_tolerance
>         raise
>       elsif errors.empty? or errors[-1].backtrace != e.backtrace
>         errors << e
>         @logger.warn { e }
>       end
>
>       retry
>     end
>   end

sure - this is fine too.

> On the broader perspective, personally I don't think having a *general*
> mechanism that automatically retries code in case of an exception being
> thrown is a good idea: whether or not you can repeat the code totally
> depends on the code at hand and I'd guess that always repeating it is not
> generally a good idea.  But then you might have multiple very similar pieces
> of code where it makes sense indeed to have some mechanism.

many, many, many peices of code.  if one is writing a production system that
runs on NFS it is unavoidable that many un-expected and rarely occuring errors
will occur.  you may think it's only ESTALE, but then it might be EAGAIN, and
then it might be EWOULDBLOCK, then the sysads mount the system with different
options and you have whole new suite of possible errors.  in short, this system
must not stop - it can have errors in the short term - but it must recover from
them and continue to run.  in fact, the code this runs also is and 'immortal'
daemon - one that restarts itself on any exit condition other than success.

cheers.

-a
--
===============================================================================
| EMAIL   :: Ara [dot] T [dot] Howard [at] noaa [dot] gov
| PHONE   :: 303.497.6469
| A flower falls, even though we love it;
| and a weed grows, even though we do not love it. 
|   --Dogen
===============================================================================