gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 26457: [19.11.x] Throw exception if update of issues table fails 

While this won't prevent the deadlock, it should catch the case where
a deadlock causes the DB update to fail and provide feedback to the user
and rollback the transaction

I don't know how to trigger the deadlock, I can only confirm that we see it, and
that this should catch it.

To test:
1 - Apply patches
2 - Checkout several items to a patron
3 - Confirm that 'Renew all' feature continues to work as expected and all items are renewed

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 26457: Unit test

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 26457: (QA follow-up) Switch to PK index in UPDATE on issues

The deadlock reports tell us that multiple transactions are
waiting for a X lock on a record but using a secondary index
on borrowernumber and itemnumber. Since we have the issue_id
at hand already, we should use that and benefit from the clustered
index (on PK) instead of using a secondary index.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
This commit is contained in:
Nick Clemens 2020-12-04 12:03:52 +00:00 committed by Victor Grousset/tuxayo
parent fc2052be19
commit 5867cf7a40
4 changed files with 44 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
C4/Circulation.pm
View File

@ -61,6 +61,7 @@ use Koha::Config::SysPrefs;
use Koha::Charges::Fees;
use Koha::Util::SystemPreferences;
use Koha::Checkouts::ReturnClaims;
use Koha::Exceptions::Checkout;
use Carp;
use List::MoreUtils qw( uniq any );
use Scalar::Util qw( looks_like_number );
@ -2961,13 +2962,17 @@ sub AddRenewal {
# Update the issues record to have the new due date, and a new count
# of how many times it has been renewed.
my $renews = $issue->renewals + 1;
my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
WHERE borrowernumber=?
AND itemnumber=?"
);
my $renews = ( $issue->renewals || 0 ) + 1;
my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? WHERE issue_id = ?");
$sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
eval{
$sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $issue->issue_id );
};
if( $sth->err ){
Koha::Exceptions::Checkout::FailedRenewal->throw(
error => 'Update of issue# ' . $issue->issue_id . ' failed with error: ' . $sth->errstr
);
}
# Update the renewal count on the item, and tell zebra to reindex
$renews = $item_object->renewals + 1;

15
Koha/Exceptions/Checkout.pm Normal file
View File

@ -0,0 +1,15 @@
package Koha::Exceptions::Checkout;
use Modern::Perl;
use Exception::Class (
'Koha::Exceptions::Checkout' => {
description => "Something went wrong!"
},
'Koha::Exceptions::Checkout::FailedRenewal' => {
isa => 'Koha::Exceptions::Checkout',
description => "Renewing checkout failed"
},
);
1;

13
svc/renew
View File

@ -21,6 +21,7 @@ use Modern::Perl;
use CGI;
use JSON qw(to_json);
use Try::Tiny;
use C4::Circulation;
use C4::Context;
@ -66,8 +67,16 @@ if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference
}
if ( $data->{renew_okay} ) {
$date_due = AddRenewal( $borrowernumber, $itemnumber, $branchcode, $date_due );
$data->{date_due} = output_pref( { dt => $date_due, as_due_date => 1 } );
try{
$date_due = AddRenewal( $borrowernumber, $itemnumber, $branchcode, $date_due );
$data->{date_due} = output_pref( { dt => $date_due, as_due_date => 1 } );
} catch {
if ( ref($_) eq 'Koha::Exceptions::Checkout::FailedRenewal' ) {
$data->{error} = 'renewal_failed';
} else {
$_->rethrow;
}
};
}
print to_json($data);

8
t/db_dependent/Circulation.t
View File

@ -19,6 +19,7 @@ use Modern::Perl;
use utf8;
use Test::More tests => 47;
use Test::Exception;
use Test::MockModule;
use Data::Dumper;
@ -3099,7 +3100,7 @@ subtest 'AddReturn should clear items.onloan for unissued items' => sub {
subtest 'AddRenewal and AddIssuingCharge tests' => sub {
plan tests => 11;
plan tests => 12;
t::lib::Mocks::mock_preference('item-level_itypes', 1);
@ -3142,6 +3143,11 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub {
# Check the item out
AddIssue( $patron->unblessed, $item->barcode );
throws_ok {
AddRenewal( $patron->borrowernumber, $item->itemnumber, $library->id, undef, {break=>"the_renewal"} );
} 'Koha::Exceptions::Checkout::FailedRenewal', 'Exception is thrown when renewal update to issues fails';
t::lib::Mocks::mock_preference( 'RenewalLog', 0 );
my $date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } );
my %params_renewal = (