Closed Bug 530098 Opened 15 years ago Closed 15 years ago

gloda deletion processing is more expensive than it needs to be

Categories

(MailNews Core :: Database, defect)

defect
Not set
major

Tracking

(blocking-thunderbird3.1 beta2+, thunderbird3.1 beta2-fixed, thunderbird3.0 .5-fixed)

RESOLVED FIXED
Thunderbird 3.1b2
Tracking Status
blocking-thunderbird3.1 --- beta2+
thunderbird3.1 --- beta2-fixed
thunderbird3.0 --- .5-fixed

People

(Reporter: asuth, Assigned: asuth)

References

Details

(Keywords: perf, Whiteboard: [gloda key])

Attachments

(2 files)

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.
Whiteboard: [gloda key]
Keywords: perf
if the potential for improvement is as good as suggested in email, we probably want this for 3.0
Severity: normal → major
blocking-thunderbird3.0: --- → ?
blocking-thunderbird3.1: --- → ?
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...
blocking-thunderbird3.1: ? → rc1+
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
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.
blocking-thunderbird3.0: ? → ---
Blocks: 547950
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.
Attachment #436896 - Flags: review?(bienvenu)
Attachment #436896 - Flags: review?(bienvenu) → review+
Status: ASSIGNED → RESOLVED
blocking-thunderbird3.1: rc1+ → beta2+
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
Attachment #436896 - Attachment description: v1 make messageAttribute deletion faster by bringing back the index we thought was already there → 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 #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.
I am experiencing the same problem. My email freezes for roughly 30 seconds after hitting the "delete" button. Exactly how do I fix this?
(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.
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 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.
Attachment #436896 - Flags: approval-thunderbird3.0.5?
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.)
Attachment #442603 - Flags: review?(bienvenu)
Attachment #442603 - Flags: review?(bienvenu) → review+
Attachment #436896 - Flags: approval-thunderbird3.0.5?
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 on attachment 442603 [details] [diff] [review] v1 mozilla-1.9.1 version of der patchen a=Standard8
Attachment #442603 - Flags: approval-thunderbird3.0.5+
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
(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
(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...)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: