[Bug 14057] New: Inventory is painfully slow
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Bug ID: 14057 Summary: Inventory is painfully slow Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Tools Assignee: gmcharlt@gmail.com Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org While attempting to run an inventory, I attempted to run an inventory on a single barcode with no filters. It gave me a gateway timeout error. My goal is to speed it up. The major source of the problem is in C4::Item, where it attempts to replace the authorized value (e.g. 0) with a description (e.g. Lost -- these are hypothetical examples, so don't expect 0=Lost somewhere). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38472 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38472&action=edit Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #2 from Marc Véron <veron@veron.ch> --- It is really faster. With 30 items I got for the loop Without patch: 0.0321040153503418 With patch: 0.00271320343017578 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Marc Véron from comment #2)
It is really faster. With 30 items I got for the loop Without patch: 0.0321040153503418 With patch: 0.00271320343017578
Thanks for the numeric confirmation. Did you notice any difference in output? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38472|0 |1 is obsolete| | --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38482 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38482&action=edit Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. nested hashes of hashes hard to debug. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38482|0 |1 is obsolete| | --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38483 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38483&action=edit Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #6 from Marc Véron <veron@veron.ch> --- Created attachment 38500 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38500&action=edit [TESTPATCH] Bug 14057 - A Rough Start - Compare old and new code This patch gets values using both methods to compare the results. For testing purposes only. To test: Apply patch Compare results by commenting /uncommenting appropriate lines after line 1154 in C4/Items.pm If found one difference: Old code could not handle Withdrawn status (displays 1 instead of the text) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Marc Véron from comment #6)
If found one difference: Old code could not handle Withdrawn status (displays 1 instead of the text) 952$0 means $sf=0, but is defined. See bug 14061.
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14061 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #8 from Marc Véron <veron@veron.ch> --- Created attachment 38506 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38506&action=edit Bug 14057 - A Rough Start - Compare old and new code This patch gets values using both methods to compare the results. For testing purposes only. To test: Apply patch Compare results by commenting /uncommenting appropriate lines after line 1154 in C4/Items.pm If found one difference: Old code could not handle Withdrawn status (displays 1 instead of the text) ------ Amended: I changed the test code to reflect the old behaviour fixed with patch from Bug 14061 It writes a warning if the old and the new value are not the same. I still got differences with the Withdrawn status and found out, that it took the auth value for OPAC. This is due to a part of the select statement in the first patch (Line 1122) that overrules the value for the staff client: IF(TB.lib_opac>'',TB.lib_opac,TB.lib) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38500|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38506|0 |1 is obsolete| | --- Comment #9 from Marc Véron <veron@veron.ch> --- Comment on attachment 38506 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38506 Bug 14057 - A Rough Start - Compare old and new code Sorry, attached wrong patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #10 from Marc Véron <veron@veron.ch> --- Created attachment 38507 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38507&action=edit Bug 14057 - A Rough Start - Compare old and new code This patch gets values using both methods to compare the results. For testing purposes only. To test: Apply patch Compare results by commenting /uncommenting appropriate lines after line 1154 in C4/Items.pm If found one difference: Old code could not handle Withdrawn status (displays 1 instead of the text) ------ Amended: I changed the test code to reflect the old behaviour fixed with patch from Bug 14061 It writes a warning if the old and the new value are not the same. I still got differences with the Withdrawn status and found out, that it took the auth value for OPAC. This is due to a part of the select statement in the first patch (Line 1122) that overrules the value for the staff client: IF(TB.lib_opac>'',TB.lib_opac,TB.lib) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #11 from Marc Véron <veron@veron.ch> --- Update: The reason was not in the select statment, but in line 1128; $row->{OPAC} should be: $row->{Intranet} I replaced it in my test code, result: New version provides the same results as old version. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14063 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #12 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Marc Véron from comment #11)
Update: The reason was not in the select statment, but in line 1128;
$row->{OPAC} should be: $row->{Intranet}
I replaced it in my test code, result: New version provides the same results as old version.
The problem is the context is unknown for the second function, so opac is always used with staff client fall-back. If I recall correctly. Not looking at code right now. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38483|0 |1 is obsolete| | --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38525 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38525&action=edit Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. Moved the SQL select to C4::Koha. Changed the GetItemsForInventory to have a hashref parameter. Added interface, in case there is a need for 'opac' vs. 'staff'. Untested. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38507|0 |1 is obsolete| | --- Comment #14 from Marc Véron <veron@veron.ch> --- Comment on attachment 38507 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38507 Bug 14057 - A Rough Start - Compare old and new code Obsoleting my temporary patch with test code -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38525|0 |1 is obsolete| | --- Comment #15 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38621 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38621&action=edit Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. Moved the SQL select to C4::Koha. Changed the GetItemsForInventory to have a hashref parameter. Added interface, in case there is a need for 'opac' vs. 'staff'. Added t/db_dependent/Items/GetItemsForInventory.t Added t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38621|0 |1 is obsolete| | --- Comment #16 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38622 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38622&action=edit Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. Moved the SQL select to C4::Koha. Changed the GetItemsForInventory to have a hashref parameter. Added interface, in case there is a need for 'opac' vs. 'staff'. Added t/db_dependent/Items/GetItemsForInventory.t Added t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t I see what you mean about opac vs. staff now. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - low |P3 --- Comment #17 from M. Tompsett <mtompset@hotmail.com> --- I'm up'ing the priority, since I have seen a user run inventory 4 times unfiltered, and it blew up their low-memory (1GB RAM) system with just 80MB of Koha DB. If dcook, mveron, or perhaps anyone else could look at this one more time, I'll submit a patch for sign off if there is no feedback in 48 hours. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #18 from M. Tompsett <mtompset@hotmail.com> --- I know there is no test plan, but if someone wishes to test it and sign it off with no test plan, why not? :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #19 from Marc Véron <veron@veron.ch> --- Created attachment 38636 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38636&action=edit [Signed-off] Bug 14057 - A Rough Start This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. Moved the SQL select to C4::Koha. Changed the GetItemsForInventory to have a hashref parameter. Added interface, in case there is a need for 'opac' vs. 'staff'. Added t/db_dependent/Items/GetItemsForInventory.t Added t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t --- How I tested: - Apply patch - Do some full and some filtered Inventories, behaviour is the same (but faster as without patch) - Run both tests Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38622|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38636|0 |1 is obsolete| | --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 38733 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38733&action=edit [PASSED QA] Bug 14057 - Inventory is painfully slow This patch is a rough start. I believe it runs exponentially faster, but its equality to the previous version needs to be tested before I clean it up to acceptable standards. Nested hashes of hashes was being a debugging nightmare. Moved the SQL select to C4::Koha. Changed the GetItemsForInventory to have a hashref parameter. Added interface, in case there is a need for 'opac' vs. 'staff'. Added t/db_dependent/Items/GetItemsForInventory.t Added t/db_dependent/Koha/GetKohaAuthorisedValuesMapping.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12025 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Delightfully pushed to master. Thanks Mark! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Tom Misilo <misilot@fit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misilot@fit.edu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 --- Comment #22 from David Cook <dcook@prosentient.com.au> --- Just noticed some typos in the POD Checkout "stautshash" below. It should be "statushash" and it should have a comma after $statushash at the end. Probably worth fixing in the event of someone copying and pasting, but not really a bug... =head2 GetItemsForInventory ($itemlist, $iTotalRecords) = GetItemsForInventory( { minlocation => $minlocation, maxlocation => $maxlocation, location => $location, itemtype => $itemtype, ignoreissued => $ignoreissued, datelastseen => $datelastseen, branchcode => $branchcode, branch => $branch, offset => $offset, size => $size, stautshash => $statushash interface => $interface, } ); -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14057 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=15027 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org