https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Severity|normal |major -- 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=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |katrin.fischer@bsz-bw.de --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am not sure if it's the right fix, but everything still seems to work alright. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64803|0 |1 is obsolete| | --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 65625 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65625&action=edit [SIGNED OFF] Bug 18725 - Process_message_queue sends duplicate emails if message_queue is not writable. Last week, we had a database server whose disk filled, causing database writes to fail. This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared. I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'. I have no good idea on how to test this patch so this is what I've got: Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + my $lock_sth = $dbh->prepare('LOCK TABLES message_queue WRITE'); + my $attempts = 0; + until ( $sth->execute() ) { + $attempts++; + die 'Unable to lock message_queue to update message status: ' . $dbh->errstr() if $attempts > 10; + sleep 1; + } + my $result = $sth->execute( $params->{'status'}, $params->{'message_id'} ); Hey, you are executing on $sth while you probably meant $lock_sth. You only prepare the lock, and start fake updating with two undefs. Overall, this seems not a good idea to me. We had problems with locking tables already and you should not lock when we run Letters.t since the lock does a commit. Note that we had a test for prove and an env variable KOHA_NO_TABLE_LOCKS. If the disk is full, a lot of other jobs must fail too btw. Doesnt the job crash on trying the update btw, or did it only send the same one mail each time? If you cant write to disk, perhaps you could try to save this error condition to the cache in memory ?? Note that processing mail will probably soon fail too without much disk space.. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 David Bourgault <david.bourgault@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david.bourgault@inlibro.com --- Comment #8 from David Bourgault <david.bourgault@inlibro.com> --- It would be easier if you could provide maybe a hint on how to block access to the message_queue table? Not sure how to test this otherwise. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- While the rebase was easy enough to do, I still couldn't figure out how to get step 2 to happen. When I locked the table, the canary code paused the script execution until I unlocked it. I did try filling the message_queue table. Somewhere around 1.5 million messages, and it complained the table was full in sql. However, that doesn't prevent an update. And the number of records was a function of how much space was left. I attempted to 'dd' the drive full, and yet I was still able to run the script. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68603|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 70805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70805&action=edit Bug 18725 - Process_message_queue sends duplicate emails if message_queue is not writable. Last week, we had a database server whose disk filled, causing database writes to fail. This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared. I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'. Test Plan: 1) Apply this patch 2) Make the message_queue table unwriteable somehow 3) Run process_message_queue.pl 4) Script should die with error -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70805|0 |1 is obsolete| | --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 70826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70826&action=edit Bug 18725 - Process_message_queue sends duplicate emails if message_queue is not writable. Last week, we had a database server whose disk filled, causing database writes to fail. This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared. I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'. Test Plan: 1) Apply this patch 2) Make the message_queue table unwriteable somehow 3) Run process_message_queue.pl 4) Script should die with error Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70826|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 70853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70853&action=edit Bug 18725 - check for writability before sending each message This will mean that if we fill the disk in the middle of processing the message queue we will not continue to send messages even if the database was writable at the time of launch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70853|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 70876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70876&action=edit Bug 18725 - Process_message_queue sends duplicate emails if message_queue is not writable. Last week, we had a database server whose disk filled, causing database writes to fail. This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared. I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'. Test Plan: 1) Apply this patch 2) Make the message_queue table unwriteable somehow 3) Run process_message_queue.pl 4) Script should die with error -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70876|0 |1 is obsolete| | --- Comment #17 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 73277 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73277&action=edit Bug 18725 - Process_message_queue sends duplicate emails if message_queue is not writable. Last week, we had a database server whose disk filled, causing database writes to fail. This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared. I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'. Test Plan: 1) Apply this patch 2) Make the message_queue table unwriteable somehow 3) Run process_message_queue.pl 4) Script should die with error Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 M. Tompsett <mtompset@hotmail.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=18725 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73277|0 |1 is obsolete| | Attachment #73278|0 |1 is obsolete| | --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 73749 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73749&action=edit Bug 18725 - Process_message_queue sends duplicate emails if message_queue is not writable. Last week, we had a database server whose disk filled, causing database writes to fail. This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared. I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'. Test Plan: 1) Apply this patch 2) Make the message_queue table unwriteable somehow 3) Run process_message_queue.pl 4) Script should die with error Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 73751 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73751&action=edit Bug 18725: (QA follow-up) Restore status in Letters, adjust test We need to restore the pending status. Otherwise we will loose some messages. (Just look at the code.) Correct number of tests, do not rely on number processed. The return of SendQueuedMessage is unreliable (just look again at the code). Viewing this as the number of processed/sent messages seems a bit useless. It merely serves as an indication now if we started processing. Test plan: Run Letters.t again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Process_message_queue sends |process_message_queue.pl |duplicate emails if |sends duplicate emails if |message_queue is not |message_queue is not |writable |writable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #23)

Why not simply:

return unless $message_object->status('pending')->store;

That does not trigger an update unless you mark a field as 'dirty' (a DBIx method that is not supported by Koha::Object and is not passed thru either). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #25)

(In reply to Marcel de Rooy from comment #24)

...

(In reply to Jonathan Druart from comment #23)

...

Why not simply:

return unless $message_object->status('pending')->store;

That does not trigger an update unless you mark a field as 'dirty' (a DBIx method that is not supported by Koha::Object and is not passed thru either).

Indeed, but... the tests pass with this change.

Yeah, we are testing if sub { return; } indeed returns undef ;) So the test could perhaps be improved. Writing to a readonly database, or whatever? No blocker for me though.. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73750|0 |1 is obsolete| | Attachment #73751|0 |1 is obsolete| | --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 74143 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74143&action=edit Bug 18725: (QA Follow-up) Use make_column_dirty instead of status change Moving the status to the invalid 'processing' might well have unwanted side-effects. (The status column will be set to empty string and we have a problem if it is not processed.) This patch allows pass-through of DBIX's make_column_dirty in Koha::Object (simple tests included) and uses it to force an update. If the update does not return true, it still exits. Test plan: [1] Read the changes. [2] Run t/db_dependent/Koha/Object.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Marcel, this change makes sense to me. However we need to return a Koha::Object instead of DBIC one, otherwise we could have weird side-effects when chaining the calls. Kyle, would you agree with this fix? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 74247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74247&action=edit Bug 18725: Test return values of make_column_dirty -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Pushed to Master |Pushed to Stable --- Comment #34 from Nick Clemens <nick@bywatersolutions.com> --- Pushed to stable for 17.11.6 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18725 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.