From: Robert Klemme <bob.news@...>
Date: 2004-08-25T03:45:41+09:00
Subject: Re: POLS - exception comparisons


<Ara.T.Howard@noaa.gov> schrieb im Newsbeitrag
news:Pine.LNX.4.60.0408241038300.16964@harp.ngdc.noaa.gov...
> 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.

Aha.

> > - 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' } }
>
> ??

Setting @fault_tolerance to 2 and comparing exceptions with != (which you
figured does not work as expected) lead to a false impression.  Try this
(btw, 2.times{} is ineffective since the first invocation throws):

   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].to_s !=
errors[-1].to_s)
           $stderr.puts e
           error_wrap errors, &block
         end
       end
     end
   end

   @fault_tolerance = 10
   error_wrap{ raise 'this is logged exactly once' }
   puts "no exception here"

Output is

$ ruby x.rb
this is logged exactly once
no exception here

Note also, that because of the recursion backtraces will never compare
equal.

> > - 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.

But it behaves differently (see above).  Apart from that because it uses
retry this code does not have the problem of changing stack traces.

> > 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.

Ah, I see.  Why don't you just use a general retry as in:

def exec_retry(tries = 3)
  begin
    yield
  rescue Exception
    tries -= 1
    raise if tries == 0
    retry
  end
end

Kind regards

    robert