Повторная попытка операции после исключения: критикуйте мой код

Мое приложение Perl использует ресурсы, которые временами становятся временно недоступными, вызывая исключения с использованием die. В частности, он обращается к базам данных SQLite, которые используются несколькими потоками и другими приложениями, использующими через DBIx::Class. Каждый раз, когда возникает такое исключение, операцию следует повторять, пока не истечет время ожидания.

Я предпочитаю лаконичный код, поэтому мне быстро надоело многократно набирать 7 лишних строк для каждой такой операции:

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

# [...]

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

... поэтому я помещаю их в функцию (для доступа к БД):

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

который я называю так:

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 } );
                }
            }
        } );
};

Есть ли лучший способ реализовать это? Я изобретаю колесо заново? Есть ли код на CPAN, который мне следует использовать?


person hillu    schedule 01.07.2009    source источник
comment
Лямбда-выражения - отличный способ улучшить повторное использование кода в подобных случаях. Хотя иногда они могут снизить удобочитаемость, в таких случаях они на самом деле значительно улучшают ее.   -  person j_random_hacker    schedule 03.07.2009
comment
Чтобы уточнить: я говорю, что то, что вы уже делаете (== лямбды), хорошо.   -  person j_random_hacker    schedule 03.07.2009


Ответы (4)


Я, наверное, был бы склонен написать retry вот так:

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 $@";
}

Он не работает идентично вашему существующему коду, но имеет несколько преимуществ.

  1. Я избавился от *10 штуки, как от другого плаката, я не мог понять его цели.
  2. эта функция может возвращать значение того, что $func() делает вызывающему.
  3. Семантически код больше похож на то, что вы делаете, по крайней мере, на мой заблуждающийся ум.
  4. _retry 0, sub { }; по-прежнему будет выполняться один раз, но никогда не будет повторять попытку, в отличие от вашей текущей версии, которая никогда не выполнит подпрограмму.

Более рекомендуемые (но немного менее рациональные) абстракции:

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 } ],
  );
};

Они также могут быть удобными дополнениями к вашему коду. (Не проверено)

person Kent Fredric    schedule 01.07.2009
comment
+1, но $ func - ›() может вернуть что-то другое в контексте списка - я бы предложил использовать wantarray () для обработки обоих случаев. - person j_random_hacker; 03.07.2009

Единственная реальная проблема, которую я вижу, - это отсутствие последнего утверждения. Вот как я бы это написал:

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;
    }
}
person Chas. Owens    schedule 01.07.2009
comment
Видимо, забыл скопировать последнюю. Это в моем исходном коде. Спасибо что подметил это. - person hillu; 02.07.2009

Я мог бы использовать «return» вместо «last» (в коде с поправками, внесенными Часом Оуэнсом), но чистый эффект тот же. Мне также непонятно, почему вы умножаете первый параметр своей функции повтора на 10.

IMNSHO, гораздо лучше (повторно) преобразовать общий скелетный код в функцию, как вы это сделали, чем постоянно писать один и тот же фрагмент кода снова и снова. Слишком велика опасность того, что:

  • Вы должны изменить логику - слишком во многих местах
  • В какой-то момент вы забываете правильно отредактировать логику

Это стандартные аргументы в пользу использования функций или эквивалентных абстракций вместо встроенного кода.

Другими словами - хорошая работа по созданию функции. И полезно, что Perl позволяет вам создавать функции на лету (спасибо, Ларри)!

person Jonathan Leffler    schedule 01.07.2009
comment
Тайм-аут выражается в секундах, и он спит 0,1 секунды между попытками, поэтому при тайм-ауте в 1 секунду будет 10 попыток. - person Chas. Owens; 02.07.2009
comment
Я согласен с тем, что исключение такого кода - хорошая идея именно по этим причинам. Мне все еще интересно, есть ли на CPAN код, обеспечивающий механизм повтора, который я мог бы просто использовать ... - person hillu; 02.07.2009

Попытка Марка Фаулера кажется довольно близкой к тому, что я описал выше. Теперь было бы удобно, если бы можно было указать какой-то фильтр исключений.

person hillu    schedule 02.07.2009