When the automatic renewal cron job is running multiple loops simultaneously, it aims to process all of the renewals for any given patron together in one chunk, in order to avoid data conflicts. To accomplish this, it starts a new data chunk each time it encounters a new patron. However, in practice, all of a patron's checkouts for a given day might not end up being consecutive in the database table if multiple patrons are using different checkout stations at the same time. (Or if differing circulation rules cause checkouts from different days to auto-renew at the same time.) If a patron's checkouts are not all consecutive in the database, they will end up being split over separate chunks. This causes digest notices not to generate correctly - depending on how the timing works out, a patron may get their renewals split over multiple digests, some renewed items may be missing from digests, or the digest may not generate at all. Sorting the issues by borrowernumber before splitting them into chunks for processing should solve this.
Created attachment 172705 [details] [review] Bug 38156: [DO NOT PUSH] Test data to reproduce the issue
Created attachment 172706 [details] [review] Bug 38156: Sort issues by borrowernumber before parallel chunking When the automatic renewal cron job is using parallel processing, it aims to process all of the renewals for any given patron together in one chunk to avoid data conflicts. To accomplish this, it starts a new data chunk each time it encounters a new patron. However, if a patron's renewing checkouts aren't all consecutive in the database, that patron's data ends up split across multiple chunks. We need to sort the issues by borrowernumber before attempting to chunk them in order to make sure they are chunked and processed correctly. To test (using KTD default test data): Setup: 1. Edit the default circulation rule: - Set Automatic renewal to "Yes" - Set No automatic renewal before to 3 2. Open the following patron accounts in separate tabs: - Floyd Delgado - Joyce Gaines - Edna Acosta - Mary Burton 3. Perform the following patron account edits for each of the above patrons (and keep the tabs open): - Enable automatic renewal notices, and set them to digests only - Add a value to the email field 4. Enter the kshell (ktd --shell) 5. Edit /etc/koha/sites/kohadev/koha-conf.xml, and add the following lines near the end, just above the </config> and </yazgfs> closing tags: <auto_renew_cronjob> <parallel_loops_count>2</parallel_loops_count> </auto_renew_cronjob> 6. restart_all Reproducing the issue: 7. Apply the test patch only 8. Run perl generate_checkouts.pl to generate test data 9. perl misc/cronjobs/automatic_renewals.pl -v -c --> The test patch added output that will show how the renewals were chunked into "chunk 0" and "chunk 1" for the two parallel loops. Note that the issues for each borrower are not processed nicely in one chunk, but are separated across multiple chunks and alternated with other borrowers. 10. Check the checkouts for each of the four patrons from above --> All checkouts should have renewed 11. Check the notices tab for each of the four patrons --> Notice errors in the automatic renewal digest notices. A patron's renewals may be split across multiple digests, a digest may be missing renewals, or a patron may not have received a digest at all Testing the patch: 12. Apply the second patch 13. Reset the due dates on all checkouts so that they will all be eligible for automatic renewal again: - koha-mysql kohadev - UPDATE issues SET date_due=<two days from today>; 14. perl misc/cronjobs/automatic_renewals.pl -v -c --> Note that the renewals are now correctly chunked by patron 15. Check the checkouts and notices tab for each of the four patrons --> All checkouts should have renewed, and all patrons should have a single new Auto Renewals Digest notice that correctly lists all of their renewed items
Created attachment 172763 [details] [review] Bug 38156: Sort issues by borrowernumber before parallel chunking When the automatic renewal cron job is using parallel processing, it aims to process all of the renewals for any given patron together in one chunk to avoid data conflicts. To accomplish this, it starts a new data chunk each time it encounters a new patron. However, if a patron's renewing checkouts aren't all consecutive in the database, that patron's data ends up split across multiple chunks. We need to sort the issues by borrowernumber before attempting to chunk them in order to make sure they are chunked and processed correctly. To test (using KTD default test data): Setup: 1. Edit the default circulation rule: - Set Automatic renewal to "Yes" - Set No automatic renewal before to 3 2. Open the following patron accounts in separate tabs: - Floyd Delgado - Joyce Gaines - Edna Acosta - Mary Burton 3. Perform the following patron account edits for each of the above patrons (and keep the tabs open): - Enable automatic renewal notices, and set them to digests only - Add a value to the email field 4. Enter the kshell (ktd --shell) 5. Edit /etc/koha/sites/kohadev/koha-conf.xml, and add the following lines near the end, just above the </config> and </yazgfs> closing tags: <auto_renew_cronjob> <parallel_loops_count>2</parallel_loops_count> </auto_renew_cronjob> 6. restart_all Reproducing the issue: 7. Apply the test patch only 8. Run perl generate_checkouts.pl to generate test data 9. perl misc/cronjobs/automatic_renewals.pl -v -c --> The test patch added output that will show how the renewals were chunked into "chunk 0" and "chunk 1" for the two parallel loops. Note that the issues for each borrower are not processed nicely in one chunk, but are separated across multiple chunks and alternated with other borrowers. 10. Check the checkouts for each of the four patrons from above --> All checkouts should have renewed 11. Check the notices tab for each of the four patrons --> Notice errors in the automatic renewal digest notices. A patron's renewals may be split across multiple digests, a digest may be missing renewals, or a patron may not have received a digest at all Testing the patch: 12. Apply the second patch 13. Reset the due dates on all checkouts so that they will all be eligible for automatic renewal again: - koha-mysql kohadev - UPDATE issues SET date_due=<two days from today>; 14. perl misc/cronjobs/automatic_renewals.pl -v -c --> Note that the renewals are now correctly chunked by patron 15. Check the checkouts and notices tab for each of the four patrons --> All checkouts should have renewed, and all patrons should have a single new Auto Renewals Digest notice that correctly lists all of their renewed items Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Do we have an award for Best Test Plan of the Month? I have a nomination.
Ah, the sorting was lost during a rebase, excellent spotting
Created attachment 172831 [details] [review] Bug 38156: Sort issues by borrowernumber before parallel chunking When the automatic renewal cron job is using parallel processing, it aims to process all of the renewals for any given patron together in one chunk to avoid data conflicts. To accomplish this, it starts a new data chunk each time it encounters a new patron. However, if a patron's renewing checkouts aren't all consecutive in the database, that patron's data ends up split across multiple chunks. We need to sort the issues by borrowernumber before attempting to chunk them in order to make sure they are chunked and processed correctly. To test (using KTD default test data): Setup: 1. Edit the default circulation rule: - Set Automatic renewal to "Yes" - Set No automatic renewal before to 3 2. Open the following patron accounts in separate tabs: - Floyd Delgado - Joyce Gaines - Edna Acosta - Mary Burton 3. Perform the following patron account edits for each of the above patrons (and keep the tabs open): - Enable automatic renewal notices, and set them to digests only - Add a value to the email field 4. Enter the kshell (ktd --shell) 5. Edit /etc/koha/sites/kohadev/koha-conf.xml, and add the following lines near the end, just above the </config> and </yazgfs> closing tags: <auto_renew_cronjob> <parallel_loops_count>2</parallel_loops_count> </auto_renew_cronjob> 6. restart_all Reproducing the issue: 7. Apply the test patch only 8. Run perl generate_checkouts.pl to generate test data 9. perl misc/cronjobs/automatic_renewals.pl -v -c --> The test patch added output that will show how the renewals were chunked into "chunk 0" and "chunk 1" for the two parallel loops. Note that the issues for each borrower are not processed nicely in one chunk, but are separated across multiple chunks and alternated with other borrowers. 10. Check the checkouts for each of the four patrons from above --> All checkouts should have renewed 11. Check the notices tab for each of the four patrons --> Notice errors in the automatic renewal digest notices. A patron's renewals may be split across multiple digests, a digest may be missing renewals, or a patron may not have received a digest at all Testing the patch: 12. Apply the second patch 13. Reset the due dates on all checkouts so that they will all be eligible for automatic renewal again: - koha-mysql kohadev - UPDATE issues SET date_due=<two days from today>; 14. perl misc/cronjobs/automatic_renewals.pl -v -c --> Note that the renewals are now correctly chunked by patron 15. Check the checkouts and notices tab for each of the four patrons --> All checkouts should have renewed, and all patrons should have a single new Auto Renewals Digest notice that correctly lists all of their renewed items Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Edit: tidied the code block inline (tcohen)
(In reply to Phil Ringnalda from comment #4) > Do we have an award for Best Test Plan of the Month? I have a nomination. Fully agreee, also for the problem description! Emily, you rock!
Pushed for 24.11! Well done everyone, thank you!
Doens't apply cleanly to 24.05.x, any chance for a rebase here?
Missing 24.05.x deps, so no backport.