232897 - collectstats.pl tries to delete from shadow database

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Over on line 57 collectstats.pl switches to the shadow database, then on line
479 CollectSeriesData() tries to delete some data without switching back to the
main database.  This corrupts data on systems that give Bugzilla write access to
their shadow database.  On b.m.o it just causes collectstats.pl to die, which is
what it has been doing on b.m.o every night since the upgrade last week.

Comment 1

[Gervase Markham [:gerv]]

Yes... when collectstats.pl was just doing queries, this made sense. Now, of
course, it does writes (not just deletes) and so we can't use the shadowdb any more.

My suggested course of action is just to remove the switch to the shadowdb. Does
that sound right?

Gerv

Comment 2

[Myk Melez [:myk] [@mykmelez]]

How expensive are the queries collectstats.pl runs?

Comment 3

[Gervase Markham [:gerv]]

It depends - users can choose any arbitrary query to be charted. However, unless
they are admins, they can't set the frequency to more than once a week.

An alternative implementation might be to switch between the shadow and real
databases every time one read or wrote (or to batch up the results and switch,
say, ever ten.) How expensive is switching databases?

Gerv

Comment 4

[Myk Melez [:myk] [@mykmelez]]

>How expensive is switching databases?

Not sure.  bbaetz- do you know?

Comment 5

[Bradley Baetz (:bbaetz)]

You can't switch. What you can do is get a separate db handle to the shadowdb,
and just use the appropriate one. If you're not using the shadowdb, then both
handles will be the same, so be careful of locking/ordering issues.

Comment 6

[Gervase Markham [:gerv]]

bbaetz: what sort of locking/ordering mistakes are possible?

Gerv

Comment 7

[Bradley Baetz (:bbaetz)]

No more than usual. Just that if you test where the dbhs are the same, a LOCK
WRITE in one, a SELECT in the second will work, but it won't work if you have a
shadowdb,

Comment 8

[Gervase Markham [:gerv]]

*** Bug 233403 has been marked as a duplicate of this bug. ***

Comment 9

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This works on my machine - but it needs careful review to see what would happen
when we have a shadowdb.

Gerv

Comment 10

[Bradley Baetz (:bbaetz)]

Comment on attachment 141099 [details] [diff] [review]
Patch v.1 

Hmm. We need to think about how |switch_to_*| can work with modperl...

I argued against direct accessors in bug 163290 comment 8, but I'm not so sure
any more. I wasn't really planning for this sort of usage for the shadowdb.
Anyway, thats a different issue, and this script doesn't need to care about mod
perl anyway.

Comment 11

[Gervase Markham [:gerv]]

Fixed.

Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.35; previous revision: 1.34
done

Gerv