Bug 12025 - Inventory tool fails with "Could not reads headers", or sometimes a timeout
Summary: Inventory tool fails with "Could not reads headers", or sometimes a timeout
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 7684
Blocks:
  Show dependency treegraph
 
Reported: 2014-04-02 12:19 UTC by Jason Etheridge
Modified: 2019-06-27 09:24 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
sample file (30 bytes, text/plain)
2014-04-02 12:19 UTC, Jason Etheridge
Details
Bug 12025 - Inventory tool fails with "Could not reads headers", or sometimes a timeout (4.79 KB, patch)
2014-05-21 11:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12025 - Inventory tool fails with "Could not reads headers", or sometimes a timeout (4.91 KB, patch)
2014-05-21 13:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 6254: (follow-up) work-around to fix a failing test case (1.83 KB, patch)
2014-05-28 14:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12025 - Inventory tool fails with "Could not reads headers", or sometimes a timeout (4.99 KB, patch)
2014-05-28 14:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Etheridge 2014-04-02 12:19:11 UTC
Created attachment 26759 [details]
sample file

This happens with small test files of one or two barcodes.

jason-temp@lumen:~$ od BARCODES.TXT
0000000 030063 033065 030463 030060 030060 030060 033062 031412
0000020 032460 031466 030061 030060 030060 031460 000064
0000035
jason-temp@lumen:~$ cat BARCODES.TXT
30563100000026
30563100000034jason-temp@lumen:~$

On the server side, it looks inventory.pl spins out of control, and is looping on commands like this:

write(5, "s\0\0\0\3select authorised_value fro"..., 119) = 119
read(5, "\1\0\0\1\1h\0\0\2\3def\4salt\27marc_subfield"..., 16384) = 144
poll([{fd=5, events=POLLIN|POLLPRI}], 1, 0) = 0 (Timeout)
write(5, "Y\0\0\0\3select authorised_value, li"..., 93) = 93
read(5, "\1\0\0\1\3\\\0\0\2\3def\4salt\21authorised_va"..., 16384) = 337
poll([{fd=5, events=POLLIN|POLLPRI}], 1, 0) = 0 (Timeout)
write(5, "\1\0\0\0\16", 5)              = 5
read(5, "\7\0\0\1\0\0\0\2\0\0\0", 16384) = 11
poll([{fd=5, events=POLLIN|POLLPRI}], 1, 0) = 0 (Timeout)
write(5, "\1\0\0\0\16", 5)              = 5
read(5, "\7\0\0\1\0\0\0\2\0\0\0", 16384) = 11
poll([{fd=5, events=POLLIN|POLLPRI}], 1, 0) = 0 (Timeout)

This is with Koha 3.14.03.000, but Chris Cormack confirmed that it happens with master, 3.14.1, and 3.14.5, but that it works fine on 3.12.12.  He tracked it down to this commit for this bug:

0313856d9a1a9c0c002da0aead8c8516b51e1c11
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7684

There is an email thread discussing this on the Koha list:
http://lists.katipo.co.nz/public/koha/2014-March/039092.html
http://lists.katipo.co.nz/public/koha/2014-April/thread.html#39127

With the Chrome development tools, I also see the following errors when I first load the interface:

Uncaught SyntaxError: Unexpected token ILLEGAL  inventory.pl:62
event.returnValue is deprecated. Please use the standard event.preventDefault() instead.   jquery.js:3

Let me know if there's anything else I can do to help.  Thanks!
Comment 1 Jonathan Druart 2014-04-02 12:54:06 UTC
Hello Jason,
I don't reproduce the issue against master (and even on the 3.14.x branch).
On the inventory tools page (tools/inventory.pl), I filled the "Barcode file" with a barcodes file, I clicked on submit and I did not get any error.
Could you try to reproduce on a sandbox please?
Comment 2 Galen Charlton 2014-05-19 20:42:33 UTC
This bug depends on the number of item records in the database.

When you upload a file of barcodes, and do not specify any filters on the rest of the form, it effectively runs GetItemsForInventory() such that *every* item in the database is returned.  On a large enough database, this takes so long that the script times out.

The current documentation in the manual implies that if you upload a file of barcodes, the *only* thing it does is set the date-last-seen field.  This is not entirely true; it also tries to compare the list of scanned items to what is expected to be seen in the list of items.

The work-around for libraries with a large number of item records is to, when uploading a file of barcodes, to also set the filters (e.g., library, shelving location, call number range, etc.) to specify the range of items that the file of barcodes corresponds to.
Comment 3 Kyle M Hall 2014-05-21 11:24:38 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-05-21 11:48:01 UTC
Comment on attachment 28401 [details] [review]
Bug 12025 - Inventory tool fails with "Could not reads headers", or sometimes a timeout

Review of attachment 28401 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt
@@ -99,5 @@
>          </fieldset>
>  
> -        <!-- submit button for uploaded file -->
> -        <fieldset class="action"><input type="submit" value="Submit" class="button" /></fieldset>
> -

This is a "feature". Why did you remove it?
Comment 5 Bernardo Gonzalez Kriegel 2014-05-21 13:53:29 UTC Comment hidden (obsolete)
Comment 6 Bernardo Gonzalez Kriegel 2014-05-21 13:56:10 UTC
(In reply to Jonathan Druart from comment #4)
> 
> This is a "feature". Why did you remove it?

I think the pourpose of this patch is to force the selection of a filter,
so removing this first submit makes sense, the one left is under filters.
Comment 7 Jonathan Druart 2014-05-21 14:14:13 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #6)
> (In reply to Jonathan Druart from comment #4)
> > 
> > This is a "feature". Why did you remove it?
> 
> I think the pourpose of this patch is to force the selection of a filter,
> so removing this first submit makes sense, the one left is under filters.

Yes but it causes a regression with bug 11272.
Nicole could I get your opinion on this change please?
Comment 8 Bernardo Gonzalez Kriegel 2014-05-21 14:30:49 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to Bernardo Gonzalez Kriegel from comment #6)
> > (In reply to Jonathan Druart from comment #4)
> > > 
> > > This is a "feature". Why did you remove it?
> > 
> > I think the pourpose of this patch is to force the selection of a filter,
> > so removing this first submit makes sense, the one left is under filters.
> 
> Yes but it causes a regression with bug 11272.
> Nicole could I get your opinion on this change please?

You are right. 
I tested re-adding it (first submit) and still works, generating the (scary) message
Perhaps switch to in discussion?
Comment 9 Galen Charlton 2014-05-21 19:27:53 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > (In reply to Bernardo Gonzalez Kriegel from comment #6)
> > > (In reply to Jonathan Druart from comment #4)
> > > > 
> > > > This is a "feature". Why did you remove it?
> > > 
> > > I think the pourpose of this patch is to force the selection of a filter,
> > > so removing this first submit makes sense, the one left is under filters.
> > 
> > Yes but it causes a regression with bug 11272.
> > Nicole could I get your opinion on this change please?
> 
> You are right. 
> I tested re-adding it (first submit) and still works, generating the (scary)
> message
> Perhaps switch to in discussion?

I think the regression on 11272 is acceptable.  This is because the premise of that bug is that uploading a barcode file and generating a list of items to view are completely independent operations.

They are not: if you upload a file of barcodes, the code doesn't just mark them seen, it also tries to compare them against a range of items specified by the filter fields to identify missing ones.
Comment 10 Galen Charlton 2014-05-21 19:29:03 UTC
Because of the string change, I will not be pushing it today, but unless I run into a problem during testing, I fully expect this to make it in for 3.16.1.
Comment 11 Marcel de Rooy 2014-05-28 14:04:18 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2014-05-28 14:04:57 UTC
(In reply to M. de Rooy from comment #11)
> Created attachment 28519 [details] [review] [review]

Oops. Wrong command :)
Comment 13 Marcel de Rooy 2014-05-28 14:09:17 UTC
Created attachment 28521 [details] [review]
Bug 12025 - Inventory tool fails with "Could not reads headers", or sometimes a timeout

For libraries with large numbers of records, using a file of barcodes in
the inventory tool without selecting filters to limit the catalog
selection to compare against can result in an error of "could not read
headers" or a page timeout.

When you upload a file of barcodes, and do not specify any filters on
the rest of the form, it effectively runs GetItemsForInventory() such
that *every* item in the database is returned.  On a large enough
database, this takes so long that the script times out.

The current documentation in the manual implies that if you upload a
file of barcodes, the *only* thing it does is set the date-last-seen
field.  This is not entirely true; it also tries to compare the list of
scanned items to what is expected to be seen in the list of items.

The work-around for libraries with a large number of item records is to,
when uploading a file of barcodes, to also set the filters (e.g.,
library, shelving location, call number range, etc.) to
specify the range of items that the file of barcodes corresponds to.

This patch adds a warning when a librarian attempts to upload a
barcode list file for comparison without selecting any filters
to limit the catalog selection.

Test Plan:
1) Apply this patch
2) Browse to the inventory tool
3) Click the "Choose File" button, select a file
4) Click the "Submit" button
5) You should see a warning message
6) Click "Cancel"
7) Select one of more filters
8) Click the "Submit" button
9) This time the script should proceed without triggering the warning.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described following test plan.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Tomás Cohen Arazi 2014-06-05 15:19:15 UTC
Pushed to master.

Thanks Kyle!
Comment 15 Galen Charlton 2014-06-05 15:33:57 UTC
Pushed to 3.16.x for inclusion in 3.16.1.
Comment 16 Fridolin Somers 2014-12-15 16:49:37 UTC
Pushed to 3.14.x, will be in 3.14.12.