From aaf54653a07b54fc918ac33015d1d36a4ca1b0ca Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 15 Jun 2023 01:23:34 +0000 Subject: [PATCH] Bug 31744: Run inventory.pl as CGI with keep alives to prevent timeouts This patches causes inventory.pl to run as CGI instead of PSGI, since Plack::App::CGIBin buffers the whole response before sending any data to the Apache reverse proxy. This patch also causes inventory.pl to quickly output HTTP headers, and print NULL characters roughly every 5 seconds until the HTML or CSV data is ready to be printed. This prevents HTTP forward and reverse proxies from timing out waiting for data from the inventory script, which can be very slow. In the future, we will want to move inventory.pl to use background jobs, but this change makes inventory.pl usable for staff users stuck behind HTTP forward proxies with short timeouts. Test plan: 0. Apply patch 1. cp debian/templates/apache-shared-intranet-plack.conf /etc/koha/apache-shared-intranet-plack.conf 2. cp debian/templates/apache-shared.conf /etc/koha/apache-shared.conf 3. service apache2 restart 4. Go to create a SQL report from SQL /cgi-bin/koha/reports/guided_reports.pl?phase=Create%20report%20from%20SQL 5. Save a report with the following SQL: SELECT barcode FROM items where barcode <> ''; 6. Run the report 7. Download as CSV 8. Edit the CSV and remove the "barcode" heading 9. Open F12 dev tools in browser and open the "Network" tab 10. Go to http://localhost:8081/cgi-bin/koha/mainpage.pl 11. Note that mainpage.pl has a "Content-Encoding: gzip" header 12. Go to http://localhost:8081/cgi-bin/koha/tools/inventory.pl 13. Note that inventory.pl does not have a "Content-Encoding: gzip" header, while the .js and .css files do have this header. 14. Click "Choose file" and choose your barcode CSV file 15. Uncheck "Compare barcodes list to results" 16. Check "Export to CSV file" 17. Click "Submit" 18. Note that the download initially only increases in size by 1B at a time 19. Open inventory.csv with a text editor that can show symbols like Notepad++ 20. Note approximately 7 or 8 NULL characters at start of file 21. Note that CSV loads without problems in Excel or LibreOffice (ie the NULL characters are ignored) 22. Return to your browser and uncheck "Export to CSV file" 23. Click "Submit" 24. On the "Response" tab, note approximately 7 or 8 NULL characters at start of response -- 25. For bonus points, set up a reverse proxy in front of inventory.pl with a proxy timeout of 10 seconds and run an inventory job. Note that the proxy times out without the patch, and does *not* time out with the patch. Signed-off-by: Emily Lamancusa --- .../apache-shared-intranet-plack.conf | 1 + debian/templates/apache-shared.conf | 6 +++ tools/inventory.pl | 43 ++++++++++++++++--- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/debian/templates/apache-shared-intranet-plack.conf b/debian/templates/apache-shared-intranet-plack.conf index 7f9f47e1e1..681eed5985 100644 --- a/debian/templates/apache-shared-intranet-plack.conf +++ b/debian/templates/apache-shared-intranet-plack.conf @@ -12,6 +12,7 @@ # FIXME: These scripts should be fixed so they # don't break under plack/starman ProxyPass "/cgi-bin/koha/tools/export.pl" "!" + ProxyPass "/cgi-bin/koha/tools/inventory.pl" "!" ProxyPass "/cgi-bin/koha/tools/upload-cover-image.pl" "!" ProxyPass "/cgi-bin/koha/svc/cataloguing/metasearch" "!" diff --git a/debian/templates/apache-shared.conf b/debian/templates/apache-shared.conf index b923578e0b..90d085069f 100644 --- a/debian/templates/apache-shared.conf +++ b/debian/templates/apache-shared.conf @@ -34,6 +34,12 @@ SetEnv PERL5LIB "/usr/share/koha/lib" + + + #Do not compress inventory.pl response, so it can stream keep alive data + #NOTE: To override AddOutputFilterByType in a higher section, you need to add a dummy directive here + AddOutputFilterByType DEFLATE text/plain + # Compress content with type html, text, and css, ... AddOutputFilterByType DEFLATE text/plain text/html text/xml text/css AddOutputFilterByType DEFLATE application/xml application/xhtml+xml application/rss+xml application/javascript application/x-javascript diff --git a/tools/inventory.pl b/tools/inventory.pl index 6038931f62..45cfe62d95 100755 --- a/tools/inventory.pl +++ b/tools/inventory.pl @@ -69,6 +69,21 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); +#NOTE: Turn off buffering so that keep alives go out ASAP +$|++; +my $keep_alive_start = time(); + +if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ + binmode STDOUT, ":encoding(UTF-8)"; + print $input->header( + -type => 'text/csv', + -attachment => 'inventory.csv', + ); +} +else { + output_html_with_http_headers $input, $cookie, ''; +} + my @authorised_value_list; my $authorisedvalue_categories = ''; @@ -192,6 +207,7 @@ if ( ($uploadbarcodes && length($uploadbarcodes) > 0) || ($barcodelist && length $uploadbarcodes = $barcodelist; } for my $barcode (@uploadedbarcodes) { + keep_alive(); next unless $barcode; $barcode = barcodedecode($barcode); @@ -219,6 +235,7 @@ if ( ($uploadbarcodes && length($uploadbarcodes) > 0) || ($barcodelist && length err_data => $err_data ); } foreach my $barcode (@barcodes) { + keep_alive(); if ( $qwithdrawn->execute($barcode) && $qwithdrawn->rows ) { push @errorloop, { 'barcode' => $barcode, 'ERR_WTHDRAWN' => 1 }; } else { @@ -299,6 +316,7 @@ if( @scanned_items ) { # Report scanned items that are on the wrong place, or have a wrong notforloan # status, or are still checked out. for ( my $i = 0; $i < @scanned_items; $i++ ) { + keep_alive(); my $item = $scanned_items[$i]; @@ -377,11 +395,6 @@ if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ eval {use Text::CSV ();}; my $csv = Text::CSV->new or die Text::CSV->error_diag (); - binmode STDOUT, ":encoding(UTF-8)"; - print $input->header( - -type => 'text/csv', - -attachment => 'inventory.csv', - ); my $columns = Koha::Database::Columns->columns; my @translated_keys; @@ -438,7 +451,13 @@ if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ exit; } -output_html_with_http_headers $input, $cookie, $template->output; +#NOTE: This is a workaround until we can refactor inventory to run in the background +#NOTE: Output headers earlier so that we can send data out progressively to keep alive +#long running operations +#output_html_with_http_headers $input, $cookie, $template->output; +my $data = $template->output; +$data =~ s/\&\;amp\; /\&\; /g; +print $data; sub additemtoresults { my ( $item, $results ) = @_; @@ -458,3 +477,15 @@ sub additemtoresults { # since the script appends to $item, we can just overwrite the hash entry $results->{$itemno} = $item; } + +#NOTE: Print data to client so proxies don't time out +sub keep_alive { + my $current_time = time(); + my $elapsed = $current_time - $keep_alive_start; + #Only print a keep alive once every 5 seconds + if ( $elapsed >= 5 ){ + print chr(0); + #Reset variable for comparing time elapsed + $keep_alive_start = $current_time; + } +} -- 2.34.1