[Bug 36901] New: background_jobs_worker.pl sets the status to 'failed' right away
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Bug ID: 36901 Summary: background_jobs_worker.pl sets the status to 'failed' right away Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org It should be logging about the error on the `catch` block. I think it should be done on top of bug 36900, and also using Koha::Logger (i.e. both). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Depends on| |36900 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36900 [Bug 36900] Job errors should be stored on a separate location -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- We are actually not supposed to hit that. If the job is correctly written it will deal with the different situations and log when need to be logged. But yes, we never know and we should at least log the exception to the log file. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #1)
We are actually not supposed to hit that.
If the job is correctly written it will deal with the different situations and log when need to be logged.
But yes, we never know and we should at least log the exception to the log file.
Yes. All jobs that are core to Koha implement a try/catch block, but ultimately call ` $self->finish( $data );` which can fail! Something to revisit, as the job might have been successful but the worker marking it as failed because of DB connection issues. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 166969 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166969&action=edit [DO NOT PUSH] Bug 36901: Simulate failure for testing purposes -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 166970 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166970&action=edit Bug 36901: Add logging for uncaught exceptions in background job classes This patch adds logging of unhandled exceptions that could occur. This is happening on busy production sites right now. This is also useful for plugin jobs that might not be 100% following the guidelines and would benefit from this. But as the [DO NOT PUSH] patch highlights, this is something we really want to have on our current codebase, as a database connection drop might make us reach that `catch` block we are adding logging to on this patch. To test: 1. Apply the [DO NOT PUSH] patch 2. Run: $ ktd --shell k$ restart_all ; tail -f /var/log/koha/kohadev/worker*.log 3. Pick a valid barcode on the staff UI 4. Use the 'Batch delete items' tool in the cataloguing section 5. Start the job for deleting the item => FAIL: The item got deleted, but the job marked as failed and no logs about the reasons 6. Apply this patch and repeat 2-5 => SUCCESS: Same scenario but there's a log with the error message 7. Sign off :-D -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35092 Depends on|36900 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35092 [Bug 35092] [OMNIBUS] Remaining background job/worker issues https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36900 [Bug 36900] Job errors should be stored on a separate location -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|background_jobs_worker.pl |Add logging for uncaught |sets the status to 'failed' |exceptions in background |right away |job classes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisette.scheer@bywatersolut | |ions.com Text to go in the| |This patch adds logging for release notes| |uncaught exceptions in | |background job classes. | |Some rare situations like | |DB connection drops can | |make jobs get marked as | |failure, but no information | |about the reasons is logged | |anywhere. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 166992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166992&action=edit Bug 36901: Simulate failure for testing purposes Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166970|0 |1 is obsolete| | --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 166993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166993&action=edit Bug 36901: Add logging for uncaught exceptions in background job classes This patch adds logging of unhandled exceptions that could occur. This is happening on busy production sites right now. This is also useful for plugin jobs that might not be 100% following the guidelines and would benefit from this. But as the [DO NOT PUSH] patch highlights, this is something we really want to have on our current codebase, as a database connection drop might make us reach that `catch` block we are adding logging to on this patch. To test: 1. Apply the [DO NOT PUSH] patch 2. Run: $ ktd --shell k$ restart_all ; tail -f /var/log/koha/kohadev/worker*.log 3. Pick a valid barcode on the staff UI 4. Use the 'Batch delete items' tool in the cataloguing section 5. Start the job for deleting the item => FAIL: The item got deleted, but the job marked as failed and no logs about the reasons 6. Apply this patch and repeat 2-5 => SUCCESS: Same scenario but there's a log with the error message 7. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This patch adds logging for |This enhancement adds release notes|uncaught exceptions in |logging for uncaught |background job classes. |exceptions in background |Some rare situations like |job classes. Some rare |DB connection drops can |situations like DB |make jobs get marked as |connection drops can make |failure, but no information |jobs get marked as failure, |about the reasons is logged |but no information about |anywhere. |the reasons is logged | |anywhere. CC| |david@davidnind.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166993|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 168245 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=168245&action=edit Bug 36901: Add logging for uncaught exceptions in background job classes This patch adds logging of unhandled exceptions that could occur. This is happening on busy production sites right now. This is also useful for plugin jobs that might not be 100% following the guidelines and would benefit from this. But as the [DO NOT PUSH] patch highlights, this is something we really want to have on our current codebase, as a database connection drop might make us reach that `catch` block we are adding logging to on this patch. To test: 1. Apply the [DO NOT PUSH] patch 2. Run: $ ktd --shell k$ restart_all ; tail -f /var/log/koha/kohadev/worker*.log 3. Pick a valid barcode on the staff UI 4. Use the 'Batch delete items' tool in the cataloguing section 5. Start the job for deleting the item => FAIL: The item got deleted, but the job marked as failed and no logs about the reasons 6. Apply this patch and repeat 2-5 => SUCCESS: Same scenario but there's a log with the error message 7. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Attachment #166992|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166969|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com Severity|enhancement |minor --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice improvement.. I'd go so far as to say it's a minor bugfix to aid developers lives. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.11.00 |24.11.00,24.05.06 released in| | CC| |lucas@bywatersolutions.com Status|Pushed to main |Pushed to stable --- Comment #10 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Backported to 24.05.x for upcoming 24.05.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|24.11.00,24.05.06 |24.11.00,24.05.06,23.11.11 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This enhancement adds |This fixes the logging of release notes|logging for uncaught |uncaught exceptions in |exceptions in background |background jobs. Some rare |job classes. Some rare |situations like DB |situations like DB |connection drops can make |connection drops can make |jobs get marked as failure, |jobs get marked as failure, |but no information about |but no information about |the reasons was logged |the reasons is logged |anywhere. |anywhere. | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36901 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldstable |RESOLVED --- Comment #12 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org