Retrying an operation after an exception: Please criticize my code.

Question

My Perl application uses resources that become temporarily unavailable at times, causing exceptions using die. Most notably, it accesses SQLite databases that are shared by multiple threads and with other applications using through DBIx::Class. Whenever such an exception occurs, the operation should be retried until a timeout has been reached.

I prefer concise code, therefore I quickly got fed up with repeatedly typing 7 extra lines for each such operation:

use Time::HiRes 'sleep';
use Carp;

# [...]

for (0..150) {
    sleep 0.1 if $_;
    eval {
        # database access
    };
    next if $@ =~ /database is locked/;
}
croak $@ if $@;

... so I put them into a (DB access-specific) function:

sub _retry {
    my ( $timeout, $func ) = @_;
    for (0..$timeout*10) {
        sleep 0.1 if $_;
        eval { $func->(); };
        next if $@ =~ /database is locked/;
    }
    croak $@ if $@;
}

which I call like this:

my @thingies;
_retry 15, sub {
    $schema->txn_do(
        sub {
            @thingies = $thingie_rs->search(
                { state => 0, job_id => $job->job_id },
                { rows  => $self->{batchsize} } );
            if (@thingies) {
                for my $thingie (@thingies) {
                    $thingie->update( { state => 1 } );
                }
            }
        } );
};

Is there a better way to implement this? Am I re-inventing the wheel? Is there code on CPAN that I should use?

Answer 1

The only real problem I see is the lack of a last statement. This is how I would write it:

sub _retry {
    my ($timeout, $func) = @_;
    for my $try (0 .. $timeout*10) {
        sleep 0.1 if $try;
        eval { $func->(); 1 } or do {
            next if $@ =~ /database is locked/; #ignore this error
            croak $@;                           #but raise any other error
        };
        last;
    }
}

Answer 2

I might use 'return' instead of 'last' (in the code as amended by Chas Owens), but the net effect is the same. I am also not clear why you multiply the first parameter of your retry function by 10.

IMNSHO, it is far better to (re)factor common skeletal code into a function as you have done than to continually write the same code fragment over and over. There's too much danger that:

• You have to change the logic - in far too many places
• You forget to edit the logic correctly at some point

These are standard arguments in favour of using functions or equivalent abstractions over inline code.

In other words - good job on creating the function. And it is useful that Perl allows you to create the functions on the fly (thanks, Larry)!

Answer 3

I'd probably be inclined to write retry like this:

sub _retry {
    my ( $retrys, $func ) = @_;
    attempt: {
      my $result;

      # if it works, return the result
      return $result if eval { $result = $func->(); 1 };

      # nah, it failed, if failure reason is not a lock, croak
      croak $@ unless $@ =~ /database is locked/;

      # if we have 0 remaining retrys, stop trying.
      last attempt if $retrys < 1;

      # sleep for 0.1 seconds, and then try again.
      sleep 0.1;
      $retrys--;
      redo attempt;
    }

    croak "Attempts Exceeded $@";
}

It doesn't work identically to your existing code, but has a few advantages.

1. I got rid of the *10 thing, like another poster, I couldn't discern its purpose.
2. this function is able to return the value of whatever $func() does to its caller.
3. Semantically, the code is more akin to what it is you are doing, at least to my deluded mind.
4. _retry 0, sub { }; will still execute once, but never retry, unlike your present version, that will never execute the sub.

More suggested ( but slightly less rational ) abstractions:

sub do_update {
  my %params = @_;
  my @result;

  $params{schema}->txn_do( sub {
      @result = $params{rs}->search( @{ $params{search} } );
      return unless (@result);
      for my $result_item (@result) {
        $result_item->update( @{ $params{update} } );
      }
  } );
  return \@result;
}

my $data = _retry 15, sub {
  do_update(
    schema => $schema,
    rs     => $thingy_rs,
    search => [ { state => 0, job_id => $job->job_id }, { rows => $self->{batchsize} } ],
    update => [ { state => 1 } ],
  );
};

These might also be handy additions to your code. ( Untested )

Answer 4

Attempt by Mark Fowler seems to be pretty close to what I described above. Now, it would be handy if one could specify some sort of exception filter.