| Summary: | Record with 1000+ holds and unique priorities causes a 500 error | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
| Component: | Hold requests | Assignee: | Lucas Gass (lukeg) <lucas> |
| Status: | Pushed to stable --- | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | fridolin.somers, gmcharlt |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.05.00,23.11.06
|
|
| Circulation function: | |||
| Attachments: |
Bug 36797: Change WHILE loop into FOREACH
Bug 36797: Change WHILE loop into FOREACH Bug 36797: Change WHILE loop into FOREACH |
||
If you're using k-t-d to test you can use this script: https://github.com/ricofreak/handy_koha_scripts/blob/main/make_holds.pl After you run it and have at least 1000 holds I then did the following inside k-t-d: sudo koha-mysql kohadev update reserves set priority = reserve_id; Maybe this can just be a FOREACH loop? It won't die after 1000 iterations. Created attachment 166302 [details] [review] Bug 36797: Change WHILE loop into FOREACH To test: 1. Find a record that has 1000 non-waiting holds with different priorities. 2. Try loading the reserve page for that record ( /reserve/request.pl?biblionumber=X ) 3. Error: Template process failed: undef error - WHILE loop terminated (> 1000 iterations) 4. APPLY PATCH 5. Try again, this time the page should load. 6. Try testing on a record with less than 1000 holds, making sure the priorities are still set right. 7. Make sure you can change your priorities and everything works right. Created attachment 166327 [details] [review] Bug 36797: Change WHILE loop into FOREACH To test: 1. Find a record that has 1000 non-waiting holds with different priorities. 2. Try loading the reserve page for that record ( /reserve/request.pl?biblionumber=X ) 3. Error: Template process failed: undef error - WHILE loop terminated (> 1000 iterations) 4. APPLY PATCH 5. Try again, this time the page should load. 6. Try testing on a record with less than 1000 holds, making sure the priorities are still set right. 7. Make sure you can change your priorities and everything works right. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Simple patch converting WHILE to FOREACH.. works as expected. Passing QA oof.. Signing off I mean ;) Created attachment 166607 [details] [review] Bug 36797: Change WHILE loop into FOREACH To test: 1. Find a record that has 1000 non-waiting holds with different priorities. 2. Try loading the reserve page for that record ( /reserve/request.pl?biblionumber=X ) 3. Error: Template process failed: undef error - WHILE loop terminated (> 1000 iterations) 4. APPLY PATCH 5. Try again, this time the page should load. 6. Try testing on a record with less than 1000 holds, making sure the priorities are still set right. 7. Make sure you can change your priorities and everything works right. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed for 24.05! Well done everyone, thank you! Pushed to 23.11.x for 23.11.06 This is going to require some rebasing for 23.05.x, I'll come back to do so ASAP. |
To recreate: 1. Find a record that has 1000 non-waiting holds with different priorities. 2. Try loading the reserve page for that record ( /reserve/request.pl?biblionumber=X ) 4. Error: Template process failed: undef error - WHILE loop terminated (> 1000 iterations) This looks to be because of TT 'While' loop in holds_table.inc: [% BLOCK priority_dropdown %] [%- SET loop_priority = 1 -%] [%- WHILE loop_priority <= last_priority -%] [%- IF this_priority == loop_priority -%] <option value="[% loop_priority | html %]" selected="selected">[% loop_priority | html %]</option> [%- ELSE -%] <option value="[% loop_priority | html %]">[% loop_priority | html %]</option> [%- END -%] [%- loop_priority = loop_priority + 1- %] [%- END -%] [% END %] TT documentation states that it will terminate a WHILE loop larger than 1000 to prevent runaways: https://template-toolkit.org/docs/manual/Directives.html#section_WHILE