[Bug 13740] New: Timeout on deleting patrons if too many patrons
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Bug ID: 13740 Summary: Timeout on deleting patrons if too many patrons Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@biblibre.com Reporter: jonathan.druart@biblibre.com QA Contact: testopia@bugs.koha-community.org C4::Borrowers::GetBorrowersToExpunge should not use a "NOT IN", it is not efficient at all. With only 1 guarantor and more than 136k patrons, the not in clause in this subroutine takes ages: mysql> select count(*) FROM borrowers where borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0) ; [...] not ended after 5min -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 36096 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36096&action=edit Bug 13740: Remove the NOT NULL clause in GetBorrowersToExpunge C4::Borrowers::GetBorrowersToExpunge should not use a "NOT IN", it is not efficient at all. With only 1 guarantor and more than 136k patrons, the not in clause in this subroutine takes ages: mysql> select count(*) FROM borrowers where borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0) ; [...] not ended after 5min With the query modified by this patch, the results come after 1 sec :) Test plan: Verify the delete_patrons.pl cronjob or the cleanborrowers tools work as before. Especially with guarantors. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@quecheelibrary.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Brendan Gallagher <brendan@bywatersolutions.com> 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=13740 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36096|0 |1 is obsolete| | --- Comment #2 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 36150 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36150&action=edit Bug 13740: Remove the NOT NULL clause in GetBorrowersToExpunge C4::Borrowers::GetBorrowersToExpunge should not use a "NOT IN", it is not efficient at all. With only 1 guarantor and more than 136k patrons, the not in clause in this subroutine takes ages: mysql> select count(*) FROM borrowers where borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0) ; [...] not ended after 5min With the query modified by this patch, the results come after 1 sec :) Test plan: Verify the delete_patrons.pl cronjob or the cleanborrowers tools work as before. Especially with guarantors. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> I had 2 guarantors/guarentees - worked ok. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Brendan Gallagher from comment #2)
Created attachment 36150 [details] [review] Bug 13740: Remove the NOT NULL clause in GetBorrowersToExpunge Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> I had 2 guarantors/guarentees - worked ok.
Arg, the partner has found a problem with this patch. Actually it catches the patrons *with* a guarantor, and it is not what we want here. I will submit a follow-up and switch the status back to needs signoff. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 36157 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36157&action=edit Bug 13740: Fix null vs not null in the query The previous patch did not work, only patrons *with* guanrantees were deleted! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch CC| |fridolyn.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #5 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- In this particular case, I think we should squash the 2 patches. It will be more safe for inclusion into stable branches. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Fridolin SOMERS from comment #5)
In this particular case, I think we should squash the 2 patches. It will be more safe for inclusion into stable branches.
Why is it more safe? The first patch has already been signed off, and the second patch highlights the sql error. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Koha Team AMU <koha.aixmarseille@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |koha.aixmarseille@gmail.com --- Comment #7 from Koha Team AMU <koha.aixmarseille@gmail.com> --- I try on a sandbox : Updatedatabase: Upgrade to 3.19.00.000 done (there's life after 3.18)\nUpgrade to 3.19.00.001 done (Bug 13436: Add more options to UsageStatsLibraryType)\nupgrade to 3.19.00.002 done (Bug 10753: replace __ANY__ with empty string in suggestions.branchcode)\nUpgrade to 3.19.00.003 done (Bug 1861 - Unique patrons logins not (totally) enforced)\nUpgrade to 3.19.00.004 done (Bug 13346: OpacExportOptions is now multiple)\nUpgrade to 3.19.00.005 done (Bug 13379 - Modify authorised_values.category to varchar(32))\nUpgrade to 3.19.00.006 done (Bug 11944 - Convert DB tables to utf8_unicode_ci)\nUpgrade to 3.19.00.007 done (Bug 12905: Check budget integrity: OK)\nUpgrade to 3.19.00.008 done (Bug 12601 - Add new foreign key aqorders.budget_id WARNING: temporary budget and fund have been created (search for "BACKUP_TMP"). At least one of your order was not linked to a budget)\nUpgrade to 3.19.00.009 done (Bug 13007 - Add new foreign key suggestions.budgetid)\nUpgrade to 3.19.00.010 done (Bug 5511 - SessionRestrictionByIP)\nUpgrade to 3.19.00.011 done (Bug 13417: Add permission to delete public lists)\nUpgrade to 3.19.00.012 done (Bug 13523 - Remove NOT NULL restriction on field marcxml due to mysql STRICT_TRANS_TABLES) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Koha Team AMU from comment #7)
I try on a sandbox :
And, what is the problem? Is not an error. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sandboxes@biblibre.com Status|Needs Signoff |Signed Off --- Comment #9 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Patch tested with a sandbox, by Koha Team AMU <koha.aixmarseille@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36150|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=13740 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36157|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=13740 --- Comment #10 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 36410 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36410&action=edit Bug 13740: Remove the NOT NULL clause in GetBorrowersToExpunge C4::Borrowers::GetBorrowersToExpunge should not use a "NOT IN", it is not efficient at all. With only 1 guarantor and more than 136k patrons, the not in clause in this subroutine takes ages: mysql> select count(*) FROM borrowers where borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0) ; [...] not ended after 5min With the query modified by this patch, the results come after 1 sec :) Test plan: Verify the delete_patrons.pl cronjob or the cleanborrowers tools work as before. Especially with guarantors. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Koha Team AMU <koha.aixmarseille@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #11 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 36411 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36411&action=edit Bug 13740: Fix null vs not null in the query The previous patch did not work, only patrons *with* guanrantees were deleted! Signed-off-by: Koha Team AMU <koha.aixmarseille@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 --- Comment #12 from Koha Team AMU <koha.aixmarseille@gmail.com> --- Sorry, no problem to apply the patch. Patrons are not deleted if they are guarantor or if there is a fee. So, ok for the deletion. About the massive deletion, we tried it on a sandbox, but the import tool was not efficient. We tried on our server : ok. Even if a message tell us that time is out, the deletion will end. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 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=13740 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36410|0 |1 is obsolete| | Attachment #36411|0 |1 is obsolete| | --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 36873 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36873&action=edit [PASSED QA] Bug 13740: Remove the NOT NULL clause in GetBorrowersToExpunge C4::Borrowers::GetBorrowersToExpunge should not use a "NOT IN", it is not efficient at all. With only 1 guarantor and more than 136k patrons, the not in clause in this subroutine takes ages: mysql> select count(*) FROM borrowers where borrowernumber NOT IN (SELECT guarantorid FROM borrowers WHERE guarantorid IS NOT NULL AND guarantorid <> 0) ; [...] not ended after 5min With the query modified by this patch, the results come after 1 sec :) Test plan: Verify the delete_patrons.pl cronjob or the cleanborrowers tools work as before. Especially with guarantors. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Koha Team AMU <koha.aixmarseille@gmail.com> 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=13740 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 36874 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36874&action=edit [PASSED QA] Bug 13740: Fix null vs not null in the query The previous patch did not work, only patrons *with* guanrantees were deleted! Signed-off-by: Koha Team AMU <koha.aixmarseille@gmail.com> 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=13740 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13740 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |chris@bigballofwax.co.nz --- Comment #16 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.18.x will be in 3.18.6 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org