Skip to content
Browse files

Implement cacheing for slow-to-generate reports

  • Loading branch information...
1 parent 9589eba commit 927943455e675b79b9dd7e6f442258234b33ae13 Ray Miller committed
View
61 lib/LIMS2/Report.pm
@@ -4,7 +4,7 @@ use strict;
use warnings FATAL => 'all';
use Sub::Exporter -setup => {
- exports => [ 'generator_for', 'generate_report' ]
+ exports => [ 'generator_for', 'generate_report', 'cached_report' ]
};
use Data::UUID;
@@ -13,13 +13,50 @@ use Text::CSV;
use IO::Pipe;
use Path::Class;
use Log::Log4perl qw( :easy );
+use Scalar::Util qw( blessed );
+use Fcntl qw( :DEFAULT :flock );
use LIMS2::Exception::System;
use LIMS2::Exception::Validation;
+sub _cached_report_ok {
+ my ( $work_dir, $cache_entry ) = @_;
+
+ my $report_dir = $work_dir->subdir( $cache_entry->id );
+ if ( $report_dir->stat && ! $report_dir->file('fail')->stat ) {
+ return 1;
+ }
+
+ $cache_entry->delete;
+ return;
+}
+
sub cached_report {
my %args = @_;
-
+ my $generator = generator_for( $args{report}, $args{model}, $args{params} );
+
+ # Take an exclusive lock to avoid race between interrogating table
+ # and creating cache row. This ensures we don't set off concurrent
+ # cache refreshes for the same report.
+ my $lock_file = $args{output_dir}->file('lims2.cache.lock');
+ my $lock_fh = $lock_file->open( O_RDWR|O_CREAT, oct(644) )
+ or LIMS2::Exception::System->throw( "open $lock_file failed: $!" );
+ flock( $lock_fh, LOCK_EX )
+ or LIMS2::Exception::System->throw( "flock $lock_file failed: $!" );
+
+ if ( my $in_cache = $generator->cached_report ) {
+ if ( _cached_report_ok( $args{output_dir}, $in_cache ) ) {
+ return $in_cache->id;
+ }
+ }
+
+ my $cache_entry = $generator->init_cached_report( generate_report_id() );
+ $lock_fh->close(); # End of critical code: release the lock
+
+ my $work_dir = init_work_dir( $args{output_dir}, $cache_entry->id );
+ run_in_background( $generator, $work_dir, $cache_entry );
+
+ return $cache_entry->id;
}
sub generator_for {
@@ -32,12 +69,16 @@ sub generator_for {
return $generator;
}
+sub generate_report_id {
+ return Data::UUID->new->create_str();
+}
+
sub generate_report {
my %args = @_;
my $generator = generator_for( $args{report}, $args{model}, $args{params} );
- my $report_id = Data::UUID->new->create_str();
+ my $report_id = generate_report_id();
INFO( "Generating $args{report} report $report_id" );
@@ -55,7 +96,7 @@ sub generate_report {
}
sub run_in_background {
- my ( $generator, $work_dir ) = @_;
+ my ( $generator, $work_dir, $cache_entry ) = @_;
local $SIG{CHLD} = 'IGNORE';
@@ -64,7 +105,9 @@ sub run_in_background {
if ( $pid == 0 ) { # child
Log::Log4perl->easy_init( { level => $WARN, file => $work_dir->file( 'log' ) } );
- do_generate_report( $generator, $work_dir );
+ $generator->model->clear_schema; # Force re-connect in child process
+ local $0 = 'Generate report ' . $generator->name;
+ do_generate_report( $generator, $work_dir, $cache_entry );
exit 0;
}
@@ -72,13 +115,13 @@ sub run_in_background {
}
sub run_in_foreground {
- my ( $generator, $work_dir ) = @_;
+ my ( $generator, $work_dir, $cache_entry ) = @_;
- return do_generate_report( $generator, $work_dir );
+ return do_generate_report( $generator, $work_dir, $cache_entry );
}
sub do_generate_report {
- my ( $generator, $work_dir ) = @_;
+ my ( $generator, $work_dir, $cache_entry ) = @_;
my $ok = 0;
@@ -101,11 +144,13 @@ sub do_generate_report {
write_report_name( $work_dir->file( 'name' ), $generator->name );
$work_dir->file( 'done' )->touch;
+ $cache_entry && $cache_entry->update( { complete => 1 } );
$ok = 1;
}
catch {
ERROR $_;
$work_dir->file( 'failed' )->touch;
+ $cache_entry && $cache_entry->delete;
};
return $ok;
View
4 lib/LIMS2/Report/ElectroporationProductionSummary.pm
@@ -17,6 +17,10 @@ has species => (
required => 1
);
+has '+param_names' => (
+ default => sub { [ 'species' ] }
+);
+
override _build_name => sub {
my $dt = DateTime->now();
return 'Electroporation Production Summary ' . $dt->ymd;
View
4 lib/LIMS2/Report/PlateList.pm
@@ -19,6 +19,10 @@ has plate_name => (
isa => 'Maybe[Str]'
);
+has '+param_names' => (
+ default => sub { [ 'plate_type', 'plate_name' ] }
+);
+
override _build_name => sub {
my $self = shift;
if ( $self->plate_type ) {
View
4 lib/LIMS2/Report/QcRun.pm
@@ -18,6 +18,10 @@ has qc_run => (
lazy_build => 1,
);
+has '+param_names' => (
+ default => sub { [ 'qc_run_id' ] }
+);
+
sub _build_qc_run {
my $self = shift;
View
4 lib/LIMS2/Report/QcRunSummary.pm
@@ -18,6 +18,10 @@ has qc_run => (
lazy_build => 1,
);
+has '+param_names' => (
+ default => sub { [ 'qc_run_id' ] }
+);
+
sub _build_qc_run {
my $self = shift;
View
4 lib/LIMS2/Report/VectorProductionSummary.pm
@@ -16,6 +16,10 @@ has species => (
required => 1
);
+has '+param_names' => (
+ default => sub { [ 'species' ] }
+);
+
override _build_name => sub {
my $dt = DateTime->now();
return 'Vector Production Summary ' . $dt->ymd;
View
61 lib/LIMS2/ReportGenerator.pm
@@ -5,6 +5,7 @@ use warnings FATAL => 'all';
use Moose;
use Iterator::Simple;
+use JSON;
use namespace::autoclean;
has name => (
@@ -27,6 +28,19 @@ has model => (
required => 1,
);
+has cache_ttl => (
+ is => 'ro',
+ isa => 'Str',
+ default => '8 hours'
+);
+
+has param_names => (
+ isa => 'ArrayRef[Str]',
+ traits => [ 'Array' ],
+ handles => { param_names => 'elements' },
+ default => sub { [] }
+);
+
sub _build_name {
confess( "_build_name() must be implemented by a subclass" );
}
@@ -50,6 +64,53 @@ sub boolean_str {
}
}
+sub cached_report {
+ my $self = shift;
+
+ my @cached = $self->model->schema->resultset('CachedReport')->search(
+ {
+ report_class => ref $self,
+ params => JSON->new->utf8->canonical->encode( $self->params_hash ),
+ expires => { '>' => \'current_timestamp' }
+ },
+ {
+ order_by => { -desc => 'expires' },
+ limit => 1
+ }
+ );
+
+ my @complete = grep { $_->complete } @cached;
+ if ( @complete ) {
+ return $complete[0];
+ }
+ elsif ( @cached ) {
+ return $cached[0];
+ }
+
+ return;
+}
+
+sub init_cached_report {
+ my ( $self, $report_id ) = @_;
+
+ my $cache_entry = $self->model->schema->resultset('CachedReport')->create(
+ {
+ id => $report_id,
+ report_class => ref $self,
+ params => JSON->new->utf8->canonical->encode( $self->params_hash ),
+ expires => \sprintf( 'current_timestamp + interval \'%s\'', $self->cache_ttl )
+ }
+ );
+
+ return $cache_entry;
+}
+
+sub params_hash {
+ my $self = shift;
+
+ return { map { $_ => $self->$_ } $self->param_names };
+}
+
__PACKAGE__->meta->make_immutable();
1;
View
4 lib/LIMS2/ReportGenerator/Plate.pm
@@ -51,6 +51,10 @@ has plate => (
}
);
+has '+param_names' => (
+ default => sub { [ 'plate_name' ] }
+);
+
sub plate_types {
confess( "plate_types() must be implemented by a subclass" );
}
View
4 lib/LIMS2/ReportGenerator/ProductionDetail.pm
@@ -19,6 +19,10 @@ has plate_type => (
lazy_build => 1
);
+has '+param_names' => (
+ default => sub { [ 'species', 'plate_type' ] }
+);
+
## no critic(RequireFinalReturn)
sub _build_plate_type {
LIMS2::Exception::Implementation->throw( "_build_plate_type() must be implemeted by a subclass" );
View
39 lib/LIMS2/WebApp/Controller/User/Report.pm
@@ -27,46 +27,16 @@ Catalyst Controller.
=cut
-=head1 GET /user/report/cache/sync/$REPORT
-
-Retrieve a cached report. Generate the report synchronously if there is no vaild copy in the cache.
-
-=cut
-
-sub cached_sync_report :Path( '/user/report/cache/sync' ) :Args(1) {
- my ( $self, $c, $report ) = @_;
-
- $self->assert_user_roles( 'read' );
-
- my $params = $c->request->params;
- $params->{species} ||= $c->session->{selected_species};
-
- my $report_id = LIMS2::Report::cached_report(
- model => $c->model( 'Golgi' ),
- report => $report,
- params => $params,
- output_dir => $self->report_dir,
- async => 0
- );
-
- if ( not defined $report_id ) {
- $c->flash( error_msg => 'Failed to generate report' );
- return $c->response->redirect( $c->uri_for( '/' ) );
- }
-
- return $c->forward( 'view_report', [ $report_id ] );
-}
-
-=head1 GET /user/report/cache/async/$REPORT
+=head1 GET /user/report/cache/$REPORT
Retrieve a cached report. Generate the report asynchronously if there is no vaild copy in the cache.
=cut
-sub cached_async_report :Path( '/user/report/cache/async' ) :Args(1) {
+sub cached_async_report :Path( '/user/report/cache' ) :Args(1) {
my ( $self, $c, $report ) = @_;
- $self->assert_user_roles( 'read' );
+ $c->assert_user_roles( 'read' );
my $params = $c->request->params;
$params->{species} ||= $c->session->{selected_species};
@@ -75,8 +45,7 @@ sub cached_async_report :Path( '/user/report/cache/async' ) :Args(1) {
model => $c->model( 'Golgi' ),
report => $report,
params => $params,
- output_dir => $self->report_dir,
- async => 1
+ output_dir => $self->report_dir
);
$c->stash(
View
10 root/lib/navigation.tt
@@ -33,27 +33,27 @@
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/VectorProductionSummary' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/VectorProductionSummary' ) %]">
Vector Production Summary
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/VectorProductionDetail' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/VectorProductionDetail' ) %]">
Vector Production Detail
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/FirstElectroporationProductionDetail' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/FirstElectroporationProductionDetail' ) %]">
First Allele Electroporation Detail
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/SecondElectroporationProductionDetail' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/SecondElectroporationProductionDetail' ) %]">
Second Allele Electroporation Detail
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/ElectroporationProductionSummary' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/ElectroporationProductionSummary' ) %]">
Electroporation Production Summary
</a>
</li>
View
10 root/site/user/index.tt
@@ -33,27 +33,27 @@
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/VectorProductionSummary' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/VectorProductionSummary' ) %]">
Vector Production Summary
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/VectorProductionDetail' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/VectorProductionDetail' ) %]">
Vector Production Detail
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/FirstElectroporationProductionDetail' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/FirstElectroporationProductionDetail' ) %]">
First Allele Electroporation Detail
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/SecondElectroporationProductionDetail' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/SecondElectroporationProductionDetail' ) %]">
Second Allele Electroporation Detail
</a>
</li>
<li>
- <a href="[% c.uri_for( '/user/report/async/ElectroporationProductionSummary' ) %]">
+ <a href="[% c.uri_for( '/user/report/cache/ElectroporationProductionSummary' ) %]">
Electroporation Production Summary
</a>
</li>

0 comments on commit 9279434

Please sign in to comment.
Something went wrong with that request. Please try again.