In guided_reports.pl, when exporting report results ($phase eq 'Export') all rows of data are fetched and then converted to the desired format. This may consume an unlimited amount of memory; when report results are particularly large, it may consume all available memory, leading to timed-out HTTP requests, crashes, and potentially data loss. (We have experienced Zebra index corruption as a result.) In master, reports/guided_reports.pl supports three export types -- tab-delimited, CSV, or *.ods. In each case, all data are loaded into memory and held there before any output is produced: 891 $sql = get_prepped_report( $sql, \@param_names, \@sql_params ); 892 my ($sth, $q_errors) = execute_query($sql); ... 895 if ($format eq 'tab') { 896 $type = 'application/octet-stream'; 897 $content .= join("\t", header_cell_values($sth)) . "\n"; 898 $content = Encode::decode('UTF-8', $content); 899 while (my $row = $sth->fetchrow_arrayref()) { 900 $content .= join("\t", @$row) . "\n"; 901 } 902 } else { 903 my $delimiter = C4::Context->preference('delimiter') || ','; 904 if ( $format eq 'csv' ) { ... 914 while (my $row = $sth->fetchrow_arrayref()) { 915 if ($csv->combine(@$row)) { 916 $content .= $csv->string() . "\n"; 917 } else { 918 push @$q_errors, { combine => $csv->error_diag() } ; 919 } 920 } 921 } 922 elsif ( $format eq 'ods' ) { ... 932 # Other line in Unicode 933 my $sql_rows = $sth->fetchall_arrayref(); 934 foreach my $sql_row ( @$sql_rows ) { 935 my @content_row; 936 foreach my $sql_cell ( @$sql_row ) { 937 push @content_row, Encode::encode( 'UTF8', $sql_cell ); 938 } 939 push @$ods_content, \@content_row; 940 } 941 942 # Process 943 generate_ods($ods_filepath, $ods_content); 944 945 # Output 946 binmode(STDOUT); 947 open $ods_fh, '<', $ods_filepath; 948 $content .= $_ while <$ods_fh>; 949 unlink $ods_filepath; 950 } The *.ods case is particularly problematic, because before any data is sent back to the user's browser, *three* copies of the full results are sitting in memory simultaneously -- @$sql_row, @$ods_content, and $content.
Hi Paul, can you share why you closed INVALID?
(In reply to Katrin Fischer from comment #1) > Hi Paul, can you share why you closed INVALID? I tweaked bug #23626 (memory consumption related to the charting feature) to encompass this problem, because they involve the same files and the same underlying problem -- running a report and then doing something with the results (charting or exporting) potentially consumes all available memory. Maybe I should post a comment there with the details from this ticket?
(In reply to Paul Hoffman from comment #2) > (In reply to Katrin Fischer from comment #1) > > Hi Paul, can you share why you closed INVALID? > > I tweaked bug #23626 (memory consumption related to the charting feature) to > encompass this problem, because they involve the same files and the same > underlying problem -- running a report and then doing something with the > results (charting or exporting) potentially consumes all available memory. > > Maybe I should post a comment there with the details from this ticket? You could use 'mark as duplicate' or choose 'RESOLVED MOVED' with a comment - that would link the bugs and make this clearer to people researching bugs later. Also if you leave out the # bugzilla will show a link: bug 23626
I'm reopening this as suggested by Nick Clemens in bug 23626.
What were the rationals for using a big string rather than writing directly to SDTOUT or a temporary file? Are theses assumptions still valid?
(In reply to Didier Gautheron from comment #5) > What were the rationals for using a big string rather than writing directly > to SDTOUT or a temporary file? > > Are theses assumptions still valid? It looks like it used to print out to STDOUT but it was changed in Bug 11679. After reviewing the code, I'd say it was probably a desire to make the code more readable/easier to maintain. However, it does create this performance problem. Fixing the "tab" and "csv" export should be fairly trivial, but the ODS will be harder since it's a more complex file format (ZIP compressed XML). I'll write another comment about that...
Wow looking at OpenOffice::OODoc... it hasn't been updated in over 10 years. It's amazing that it still works. Excel::Writer::XLSX has an interesting little write up on the topic of memory usage (see https://metacpan.org/pod/Excel::Writer::XLSX#SPEED-AND-MEMORY-USAGE). It looks like OpenOffice::OODoc uses Archive::Zip and Archive::Zip doesn't seem to be able to stream to output... It looks like Excel::Writer::XLSX manages memory by writing worksheets out as temporary files (https://metacpan.org/pod/Excel::Writer::XLSX#set_tempdir()). Of course Excel::Writer::XLSX still adds every one of those temporary files into memory when it's saving the workbook, so it would still have a memory spike.
Rewriting OpenOffice::OODoc is not really an option, but that would be the most "correct" solution I imagine. However, realistically, we could be more optimal in our current use of OpenOffice::OODoc. As Paul mentions, it makes no sense to do a $sth->fetchall_arrayref(), plus building @$ods_content, and then having an in-memory representation in OpenOffice::OODoc. That's 3x more memory usage than we need to use. We should just use something like $sth->fetchrow_hashref or $sth->fetchrow_arrayref and feed OpenOffice::OODoc row by row. -- Note too that the ODS format will have proxy issues for large exports, because guided_reports.pl will write the ZIP to disk, read the ZIP from disk, write to STDOUT, and then Plack::App::CGIBin will buffer STDOUT in a temporary file on disk, and then it will send the whole response all at once to the Apache reverse proxy.
An alternative solution might be to write a CSV file to a temporary file and then use LibreOffice's CLI tools to convert from CSV to ODS. Example: soffice --convert-to ods koha.csv --headless I haven't tried that though, so I can't speak to its performance. I seem to recall Indranil saying OpenOffice or LibreOffice had some memory usage issues for large spreadsheets. It might not be any better. Plus, it would add a large dependency to Koha for just 1 thing. I think that's all I have for ideas though. I think the ODS format is just problematic - at least with the tools that we have to hand.
Just a note here to say that this is still a problem - perhaps we should only support CSV and TAB for exports?
(In reply to Liz Rea from comment #10) > Just a note here to say that this is still a problem - perhaps we should > only support CSV and TAB for exports? The .ods format is what we use most as CSV is not recognized by Excel as utf8 and so umlauts are broken. You have to separately import the data which is a lot of extra steps. I'd really really like to keep it.