Whining fails if mail queueing is enabled

RESOLVED FIXED in Bugzilla 3.4

Status

()

--
critical
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: justdave, Assigned: LpSolit)

Tracking

3.4.4
Bugzilla 3.4
Bug Flags:
approval +
approval3.4 +
blocking3.4.5 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

whine.pl is still using the shadow database at the point where it tries to send mail.  This causes it to try to insert into the slave when adding to the queue, and the insert fails.

Inserting a send_mail job into the Job Queue failed with the following
error: Failed to execute INSERT INTO ts_job (funcid, arg, uniqkey,
insert_time, run_after, grabbed_until, priority, coalesce) VALUES (?,
?, ?, ?, ?, ?, ?, ?) with funcid, arg, uniqkey, insert_time, run_after,
grabbed_until, priority, coalesce: DBD::mysql::st execute failed: The
MySQL server is running with the --read-only option so it cannot
execute this statement at
/usr/lib/perl5/vendor_perl/5.8.8/Data/ObjectDriver/Driver/DBI.pm line
380.

The proper place to fix this is probably in the queue management code.  The queue insert routines should never be talking to the slave database.
Flags: blocking3.4.5?
Assignee: whining → email-notifications
Component: Whining → Email Notifications
Yeah, agreed with your suggested resolution. We should have an on_main_database routine that takes a block and performs the code using the main database.
Flags: blocking3.4.5? → blocking3.4.5-

Updated

10 years ago
Flags: blocking3.4.5- → blocking3.4.5+

Updated

10 years ago
Target Milestone: --- → Bugzilla 3.4
(Assignee)

Comment 2

10 years ago
Posted patch patch, v1 (obsolete) — Splinter Review
Create Bugzilla->main_dbh, which returns the main DB handle without switching to it.
Assignee: email-notifications → LpSolit
Status: NEW → ASSIGNED
Attachment #413823 - Flags: review?(mkanat)
Comment on attachment 413823 [details] [diff] [review]
patch, v1

Let's call it dbh_main, consistently. (It has two names in this patch, main_dbh and dbh_main.)

Also, I was thinking that instead, we'd create a function called on_main_db(&) that takes a block and makes Bugzilla->dbh be the main db for that block. I think that might be more useful in the general case, because that would also make any subroutines called use the main dbh.
Attachment #413823 - Flags: review?(mkanat) → review-
Please add a comment to JobQueue.pm explaining this.

Also, this is going to make the QueueRunner run on the main db too, but I don't think that that's a problem - certainly won't make any difference for the current job we run
(Assignee)

Comment 5

10 years ago
(In reply to comment #3)
> (From update of attachment 413823 [details] [diff] [review])
> Let's call it dbh_main, consistently. (It has two names in this patch, main_dbh
> and dbh_main.)

Haha, yes, I wrote it quickly. :)


> Also, I was thinking that instead, we'd create a function called on_main_db

This wouldn't be really useful as we already have switch_to_main_db() for that. I will just update my patch to fix the function name.
(Assignee)

Comment 6

10 years ago
Posted patch patch, v2Splinter Review
Attachment #413823 - Attachment is obsolete: true
Attachment #413934 - Flags: review?(mkanat)
(In reply to comment #5)
> This wouldn't be really useful as we already have switch_to_main_db() for that.

  It's true, but that doesn't do this:

  1) Are we running on the shadow? If so, switch to the main and remember that we were running on the shadow.
  2) At the end of the block, switch back to using the shadow, only if we were on the shadow before.

  It's not a *lot* of code, but it would simplify using the main dbh in a lot of places.

  For this bug, though, I agree, main_dbh is all we need.

Updated

10 years ago
Attachment #413934 - Flags: review?(mkanat) → review+
Comment on attachment 413934 [details] [diff] [review]
patch, v2

>Index: Bugzilla/JobQueue.pm
>+    $class->request_cache->{dbh} = $class->dbh_main;
>+    return $class->{dbh_main};

  That needs to be $class->dbh_main, on the last line there.

  Otherwise this looks fine.
(Assignee)

Comment 9

10 years ago
(In reply to comment #8)
> >+    $class->request_cache->{dbh} = $class->dbh_main;
> >+    return $class->{dbh_main};
> 
>   That needs to be $class->dbh_main, on the last line there.

This doesn't *need* to be, as all it would do would be to return ->{dbh_main}, which I do explicitly here.
Flags: approval3.4+
Flags: approval+
(In reply to comment #9)
> This doesn't *need* to be, as all it would do would be to return ->{dbh_main},
> which I do explicitly here.

  No, it does. There is no such thing as $class->{dbh_main}. $class is a string.
(Assignee)

Comment 11

10 years ago
(In reply to comment #9)
> This doesn't *need* to be, as all it would do would be to return ->{dbh_main},
> which I do explicitly here.

Ah right, I meant $class->request_cache->{dbh_main}. I will use $class->dbh_main, then.
(Assignee)

Comment 12

10 years ago
tip:

Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.84; previous revision: 1.83
done
Checking in Bugzilla/JobQueue.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/JobQueue.pm,v  <--  JobQueue.pm
new revision: 1.4; previous revision: 1.3
done


3.4.4:

Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.73.2.3; previous revision: 1.73.2.2
done
Checking in Bugzilla/JobQueue.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/JobQueue.pm,v  <--  JobQueue.pm
new revision: 1.1.2.1; previous revision: 1.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.