530098 - gloda deletion processing is more expensive than it needs to be

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

In a nutshell, deletion appears to be much slower than it has any right to be; with the database changes this is not fatal, but it is something that should be fixed.  I think even when we add in orphan processing we should still be able to be faster than we are, at least for the core deletion processing.

Myk raised a question on m.s.thunderbird about deletion, and as part of one of my responses I said:

The actual DELETE of just the message is likely pretty cheap.  The higher costs are that we try and load all the other messages in the conversation to see if they should also be purged.  That load goes through the entire ORM-ish load process (loading the related contacts/identities if the message has that data attached).  We could likely use a more precisely targeted query to answer that question rather than loading everyone into memory and checking there. (Especially since the most expensive loads are the ones that are a giveaway we don't need to purge the converation.)

It's also conceivable some of the other queries are looking at more records than they need to, but I would probably be seeing worse performance in that case.  I am hoping to have the unit tests start emitting EXPLAIN-ations of the queries soon so it's easier to sanity check that stuff.

Comment 1

[Wayne Mery (:wsmwk)]

if the potential for improvement is as good as suggested in email, we probably want this for 3.0

Comment 2

[David :Bienvenu]

Marking blocking for 3.1, since it seems like medium hanging fruit, and delete is a common operation :-) Andrew, if you don't think this should block 3.1 rc1, let me know...

Comment 3

[Mark Banner (:standard8)]

I'm saying this is wanted on 3.0.x if the fix is sensible for that branch. If it isn't then we'll wait till 3.1.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 make messageAttribute deletion faster by bringing back the index we thought was already there http://hg.mozilla.org/comm-central/rev/c87e0a6043f9

It turns out we are doing a table scan on deletion for messageAttributes.  The index I thought existed and would make this not ridiculously expensive was actually removed some time ago:
http://hg.mozilla.org/comm-central/diff/06fabe695ede/modules/datastore.js

The fix is adding an index.  This can be done without blowing away the database, so I've made the migrate logic do that.  There is a minor synchronous kick-in-the-pants at startup if you have a large database.  That could actually be optimized to be on the async path, but I don't think it's worth it for 3.1.  It might make sense for a 3.0 backport depending on how long 3.0.x is going to be the best option available to people.

I've started leaving the gaps in the schema numbers I had thought about previously so we can allow people a little bit of leeway in moving among nightly versions without exploding their gloda database every time.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/c87e0a6043f9

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to comment #4)
> It might make sense for a 3.0 backport depending on how long 3.0.x is going to
> be the best option available to people.

I vote yes because 3.1 is still at least 2 months away release (and arguably 2-4 weeks more before wide uptake is feasible) and it's a major pain point for those who see it. And equally important I think is for developers and triagers trying to diagnose performance bugs and topics, it reduces complexity by killing one item from the shrinking but still long list of possible causes to be considered.

Comment 7

[Chris]

I am experiencing the same problem.  My email freezes for roughly 30 seconds after hitting the "delete" button.  Exactly how do I fix this?

Comment 8

[Wayne Mery (:wsmwk)]

(In reply to comment #7)
> I am experiencing the same problem.  My email freezes for roughly 30 seconds
> after hitting the "delete" button.  Exactly how do I fix this?

if you want immediate relief and you have good backups you might try the current 3.1 nightly - ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-1.9.2/ - or wait till the more tested 3.1 beta 2 available in a couple weeks.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Freezing immediately after hitting the delete button is not this bug.  But it's still a good idea to try a nightly, especially as that's the first step in us being able to do something about problems.

Comment 10

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 436896 [details] [diff] [review]
v1 make messageAttribute deletion faster by bringing back the index we thought was already there http://hg.mozilla.org/comm-central/rev/c87e0a6043f9

(In reply to comment #6)
> (In reply to comment #4)
> > It might make sense for a 3.0 backport depending on how long 3.0.x is going to
> > be the best option available to people.
> 
> I vote yes because 3.1 is still at least 2 months away release (and arguably
> 2-4 weeks more before wide uptake is feasible) and it's a major pain point for
> those who see it. And equally important I think is for developers and triagers
> trying to diagnose performance bugs and topics, it reduces complexity by
> killing one item from the shrinking but still long list of possible causes to
> be considered.

Yeah, the way we're spinning the 2.0.x EOL we need the fix on 3.0.x.  I need a new patch to do this, and am going to start work on it once I hit the submit button, but I'm going to request approval on this patch for now for radar purposes...

The new patch will perform the table ALTER asynchronously to avoid an unpleasant 3.0.5 upgrade experience.

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 mozilla-1.9.1 version of der patchen

The patch has sufficient comments in there that I think it's self documenting.

To verify I:
0) (Created a new profile for use only on Thunderbird 3.0.x so I don't angry up gloda in places I care about.)
1) Ran without the patch and quit.
2) Ran sqlite3 against global-messages-db.sqlite in the profile, invoking ".schema" to see what the schema was and that it *did not* have the "messageAttribFastDeletion" index.
3) Applied the patch.
4) Ran with the patch and quit.
5) Ran sqlite3 against global-messages-db.sqlite in the profile, invoking ".schema" to see what the schema was and that it *did* have the "messageAttribFastDeletion" index.
6) Ran again and made sure the error console did not mention anything about failed SQL statements in order to verify that the logic properly detected that the index now exists and there is no need to dispatch the statement.  (Gloda will report database errors to the error console, and the creation of the index should fail when it already exists.)

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 436896 [details] [diff] [review]
v1 make messageAttribute deletion faster by bringing back the index we thought was already there http://hg.mozilla.org/comm-central/rev/c87e0a6043f9

Not on this patch ;-)

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 442603 [details] [diff] [review]
v1 mozilla-1.9.1 version of der patchen

a=Standard8

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

pushed to Thunderbird 3.0.x branch for 3.0.5 release on comm-1.9.1:
http://hg.mozilla.org/releases/comm-1.9.1/rev/e9e9219715f3

Comment 15

[Mark Banner (:standard8)]

(In reply to comment #14)
> pushed to Thunderbird 3.0.x branch for 3.0.5 release on comm-1.9.1:
> http://hg.mozilla.org/releases/comm-1.9.1/rev/e9e9219715f3

Somehow that was landed on an old relbranch (3.0rc3!). So I backed it out:

http://hg.mozilla.org/releases/comm-1.9.1/rev/be0895fabf99

and re-landed it on default:

http://hg.mozilla.org/releases/comm-1.9.1/rev/0083ce215938

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

(In reply to comment #15)
> Somehow that was landed on an old relbranch (3.0rc3!). So I backed it out:

Ugh, apologies about that.  I must have checked I was on the right branch on a different repo twice and none on that one.  Looking at my log, I did do an hg outgoing and ignored the 'branch' line (twice)... I'll be sure to sanity check that line too in the future.  (I mainly just do the outgoing to make sure I'm not leaking other commits...)