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

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 )