Attachment 98899 Details for Bug 23571 – Bug 23571: Add flock to fines.pl

[patch] Bug 23571: Add flock to fines.pl

Bug-23571-Add-flock-to-finespl.patch (text/plain), 8.04 KB, created by Hugo Agud on 2020-02-14 07:54:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Hugo Agud

Created: 2020-02-14 07:54:03 UTC

Size: 8.04 KB

patch

obsolete

>From 038b164708068db76d8a82e6f967745f6df3e4ad Mon Sep 17 00:00:00 2001
>From: Agustin Moyano <agustinmoyano@theke.io>
>Date: Thu, 5 Dec 2019 18:59:17 -0300
>Subject: [PATCH] Bug 23571: Add flock to fines.pl
>
>This patch adds a lock file for fines.pl.
>
>To test:
>1. Apply this patch
>2. In two separate consoles run misc/cronjobs/fines.pl
>SUCCESS => The first one runs normally, but the second one fails with a message flock failed for the lock file.
>3. Sign off.
>
>Sponsored-by: Orex Digital
>
>Signed-off-by: Hugo Agud <hagud@orex.es>
>---
> misc/cronjobs/fines.pl | 182 ++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 118 insertions(+), 64 deletions(-)
>
>diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl
>index f04da4b280..1e83e44d00 100755
>--- a/misc/cronjobs/fines.pl
>+++ b/misc/cronjobs/fines.pl
>@@ -36,15 +36,19 @@ use C4::Overdues;
> use Getopt::Long;
> use Carp;
> use File::Spec;
>+use Fcntl qw(:flock);
> 
> use Koha::Calendar;
> use Koha::DateUtils;
> use C4::Log;
> 
>+use constant LOCK_FILENAME => 'fines..LCK';
>+
> my $help;
> my $verbose;
> my $output_dir;
> my $log;
>+my $use_flock = 1;
> 
> GetOptions(
>     'h|help'    => \$help,
>@@ -74,6 +78,26 @@ if ($help) {
> 
> cronlogaction();
> 
>+my ($lockfile, $LockFH);
>+foreach (
>+    '/var/lock/koha',
>+    '/var/lock/koha_fines',
>+    '/tmp/koha_fines'
>+) {
>+    #we try three possibilities (we really want to lock :)
>+    next if !$_;
>+    ($LockFH, $lockfile) = _create_lockfile($_);
>+    last if defined $LockFH;
>+}
>+if( !defined $LockFH ) {
>+    print "WARNING: Could not create lock file $lockfile: $!\n";
>+    print "Verify if directories /var/lock/koha/". C4::Context->config('database').", /var/lock/koha_fines_" . C4::Context->config('database')." or /tmp/koha_fines_" . C4::Context->config('database')." exist and check file permissions\n";
>+    $use_flock = 0; # we disable file locking now and will continue
>+                    # without it
>+                    # note that this mimics old behavior (before we used
>+                    # the lockfile)
>+};
>+
> my @borrower_fields =
>   qw(cardnumber categorycode surname firstname email phone address citystate);
> my @item_fields  = qw(itemnumber barcode date_due);
>@@ -98,78 +122,83 @@ if ($filename) {
> }
> my $counted = 0;
> my $overdues = Getoverdues();
>-for my $overdue ( @{$overdues} ) {
>-    next if $overdue->{itemlost};
>-
>-    if ( !defined $overdue->{borrowernumber} ) {
>-        carp
>-"ERROR in Getoverdues : issues.borrowernumber IS NULL.  Repair 'issues' table now!  Skipping record.\n";
>-        next;
>-    }
>-    my $borrower = BorType( $overdue->{borrowernumber} );
>-    my $branchcode =
>-        ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch}
>-      : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
>-      :                                     $overdue->{branchcode};
>-
>-# In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here).
>-    if ( !exists $is_holiday{$branchcode} ) {
>-        $is_holiday{$branchcode} = set_holiday( $branchcode, $today );
>-    }
>+if(_flock($LockFH, LOCK_EX|LOCK_NB)) {
>+    for my $overdue ( @{$overdues} ) {
>+        next if $overdue->{itemlost};
>+
>+        if ( !defined $overdue->{borrowernumber} ) {
>+            carp
>+    "ERROR in Getoverdues : issues.borrowernumber IS NULL.  Repair 'issues' table now!  Skipping record.\n";
>+            next;
>+        }
>+        my $borrower = BorType( $overdue->{borrowernumber} );
>+        my $branchcode =
>+            ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch}
>+        : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
>+        :                                     $overdue->{branchcode};
>+
>+    # In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here).
>+        if ( !exists $is_holiday{$branchcode} ) {
>+            $is_holiday{$branchcode} = set_holiday( $branchcode, $today );
>+        }
> 
>-    my $datedue = dt_from_string( $overdue->{date_due} );
>-    if ( DateTime->compare( $datedue, $today ) == 1 ) {
>-        next;    # not overdue
>-    }
>-    ++$counted;
>-
>-    my ( $amount, $unitcounttotal, $unitcount ) =
>-      CalcFine( $overdue, $borrower->{categorycode},
>-        $branchcode, $datedue, $today );
>-
>-    # Don't update the fine if today is a holiday.
>-    # This ensures that dropbox mode will remove the correct amount of fine.
>-    if ( $mode eq 'production' && !$is_holiday{$branchcode} ) {
>-        if ( $amount && $amount > 0 ) {
>-            UpdateFine(
>-                {
>-                    issue_id       => $overdue->{issue_id},
>-                    itemnumber     => $overdue->{itemnumber},
>-                    borrowernumber => $overdue->{borrowernumber},
>-                    amount         => $amount,
>-                    due            => output_pref($datedue),
>-                }
>-            );
>+        my $datedue = dt_from_string( $overdue->{date_due} );
>+        if ( DateTime->compare( $datedue, $today ) == 1 ) {
>+            next;    # not overdue
>+        }
>+        ++$counted;
>+
>+        my ( $amount, $unitcounttotal, $unitcount ) =
>+        CalcFine( $overdue, $borrower->{categorycode},
>+            $branchcode, $datedue, $today );
>+
>+        # Don't update the fine if today is a holiday.
>+        # This ensures that dropbox mode will remove the correct amount of fine.
>+        if ( $mode eq 'production' && !$is_holiday{$branchcode} ) {
>+            if ( $amount && $amount > 0 ) {
>+                UpdateFine(
>+                    {
>+                        issue_id       => $overdue->{issue_id},
>+                        itemnumber     => $overdue->{itemnumber},
>+                        borrowernumber => $overdue->{borrowernumber},
>+                        amount         => $amount,
>+                        due            => output_pref($datedue),
>+                    }
>+                );
>+            }
>+        }
>+        if ($filename) {
>+            my @cells;
>+            push @cells,
>+            map { defined $borrower->{$_} ? $borrower->{$_} : q{} }
>+            @borrower_fields;
>+            push @cells, map { $overdue->{$_} } @item_fields;
>+            push @cells, $unitcounttotal, $amount;
>+            say {$fh} join $delim, @cells;
>         }
>     }
>-    if ($filename) {
>-        my @cells;
>-        push @cells,
>-          map { defined $borrower->{$_} ? $borrower->{$_} : q{} }
>-          @borrower_fields;
>-        push @cells, map { $overdue->{$_} } @item_fields;
>-        push @cells, $unitcounttotal, $amount;
>-        say {$fh} join $delim, @cells;
>+    if ($filename){
>+        close $fh;
>     }
>-}
>-if ($filename){
>-    close $fh;
>-}
> 
>-if ($verbose) {
>-    my $overdue_items = @{$overdues};
>-    print <<"EOM";
>-Fines assessment -- $today
>+    if ($verbose) {
>+        my $overdue_items = @{$overdues};
>+        print <<"EOM";
>+    Fines assessment -- $today
> EOM
>-    if ($filename) {
>-        say "Saved to $filename";
>-    }
>-    print <<"EOM";
>-Number of Overdue Items:
>-     counted $overdue_items
>-    reported $counted
>+        if ($filename) {
>+            say "Saved to $filename";
>+        }
>+        print <<"EOM";
>+    Number of Overdue Items:
>+        counted $overdue_items
>+        reported $counted
> 
> EOM
>+    }
>+    _flock($LockFH, LOCK_UN);
>+} else {
>+    print "Skipping fines assessment because flock failed on $lockfile: $!\n";
> }
> 
> sub set_holiday {
>@@ -196,3 +225,28 @@ sub get_filename {
>     }
>     return $name;
> }
>+
>+sub _create_lockfile { #returns undef on failure
>+    my $dir= shift;
>+    unless (-d $dir) {
>+        eval { mkpath($dir, 0, oct(755)) };
>+        return if $@;
>+    }
>+    return if !open my $fh, q{>}, $dir.'/'.LOCK_FILENAME;
>+    return ( $fh, $dir.'/'.LOCK_FILENAME );
>+}
>+
>+sub _flock {
>+    my ($fh, $op)= @_;
>+    return 1 if !$use_flock;
>+
>+    #check if flock is present; if not, you will have a fatal error
>+    my $lock_acquired = eval { flock($fh, $op) };
>+
>+    # assuming that $fh and $op are fine(..), an undef $lock_acquired
>+    # means no flock
>+    $use_flock = defined($lock_acquired) ? 1 : 0;
>+    print "Warning: flock could not be used!\n" if !$use_flock;
>+    return 1 if !$use_flock;
>+    return $lock_acquired;
>+}
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 23571: 96055 | 96056 | 98898 | 98899 | 99263 | 99264 | 99265 | 102772 | 102804 | 102821 | 102844 | 102888 | 102910 | 102911 | 102912