Confirmed users
360
edits
User:Andrew Sutherland/MailNews/GlobalDatabase/ReviewBelievedResolved: Difference between revisions
User:Andrew Sutherland/MailNews/GlobalDatabase/ReviewBelievedResolved (view source)
Revision as of 10:10, 27 January 2009
, 27 January 2009→bienvenu
| Line 224: | Line 224: | ||
<b>(asuth)</b> {{bug|468286}} tracks such a desire. | <b>(asuth)</b> {{bug|468286}} tracks such a desire. | ||
==== idle service use ==== | |||
<b>(bienvenu [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c15 c15] q1)</b> <i>I don't see the gloda code using the idle service to do background indexing on | |||
idle - am I missing that? Is that planned, or does it not seem like the right | |||
thing to do?</i> | |||
<b>(asuth [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c17 c17] a1)</b> Correct, the idle service is not currently used. The initial implementation | |||
just used a timer interval to schedule things, and it appears to work | |||
reasonably well. Which is, no one who has tried gloda thus far has complained | |||
about a lack of responsiveness. In practice, the introduction of the body | |||
fetching has actually slowed things down even more. | |||
My original plan was to use the idle service to control the parameters of the | |||
timer-based indexing. Those variables being: the number of milliseconds | |||
between re-scheduling, and the number of messages processed before we stop | |||
processing pending another timer interrupt (captured using a token bucket-style | |||
implementation). The idea was that when the machine was idle, we could | |||
throttle up our indexing. | |||
I think this general idea is the right idea, because we *do* want to index when | |||
the user is around, otherwise we lose the ability to keep up with new messages | |||
/ changed messages. At least, for new/changed messages. When we are just | |||
trying to get gloda up to speed, that part is not as important, but we still | |||
want to get up to speed as quickly as possible. | |||
There's also some forthcoming interaction with Emre's automatic IMAP | |||
offline-fetching stuff that needs to happen. I haven't entirely thought it | |||
through, but it seems like, to a large extent, we could just play | |||
follow-the-leader on the offline fetching... but there are a large number of | |||
cases that need to be dealt with. | |||
Your thoughts/input are greatly appreciated on this one. | |||
<b>(bienvenu [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c20 c20] p2)</b> Re using the idle service to know when to throttle up the indexing, I think | |||
that's a good plan. My brief experience with gloda and expmess is that indexing | |||
while I'm doing stuff is pretty painful, so much so that I would really | |||
throttle down the amount of stuff that's done when not idle. When new messages | |||
arrive, I would put them on a queue of messages to be indexed, and not index | |||
them right away. Similarly, if the user selects 100 messages and deletes them, | |||
we'd really like to avoid doing all the gloda housekeeping right away (it looks | |||
like you have infrastructure in place to do all this, so it's probably just a | |||
matter of being less aggressive while not idle). Also (I apologize for not | |||
fully groking the code :-) ), can/do you do the body indexing separate from the | |||
basic adding of the header to gloda? As you say, streaming the message through | |||
libmime is pretty cpu intensive. | |||
I realize there are tradeoffs between having the gloda data up to date, and | |||
keeping the UI performant, and there's always the shutdown case. It might be | |||
simplest just to remember which folders have pending operations, and sync those | |||
on the next run, instead of building full-blown persistance of your job queues. | |||
I'm on Windows, on a rather old box, but I really did feel the pain when new | |||
mail came in and they started getting indexed while I was reading and deleting | |||
them... | |||
<b>(dmose [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c22 c22])</b> Seems like chunking up the work into a finer granularity might be another way | |||
to address this. | |||
<b>(asuth current status (9/18))</b> Indexing is being re-done. | |||
<b>(asuth)</b> Indexing was re-done and the idle service does come into the picture. However, there are some issues about not being adaptive, and {{bug|470329}} tracks that. | |||
==== make integration ==== | |||
<b>(bienvenu [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c27 c27] [https://bugzilla.mozilla.org/attachment.cgi?id=337873&action=edit attached patch])</b> <i>I think you also need this patch to get gloda to build if you build from the top - I had to build from mailnews/db otherwise, to get gloda to build. I haven't finished checking that this will fix it, but I suspect it will.</i> | |||
<b>(asuth current status (9/18))</b> This patch is good and should be folded into my tree patch. | |||
<b>(asuth)</b> The patch was effectively folded in; gloda is in the tree now and knows how to build, etc. | |||
==== updateFolder usage ==== | |||
<b>(bienvenu [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c32 c32])</b> | |||
<i>One other issue I've run into - gloda seems to update folders on startup, and I | |||
run into an assertion because we're in the middle of classifying new mail as | |||
junk or not junk, because the ui is getting new mail at the same time. I've hit | |||
this for my pop3 inbox, and the assertion is in | |||
nsMsgLocalMailFolder::SpamFilterClassifyMessages. This is a potentially bad | |||
assertion because if we get out of sync with our classification requests, it | |||
can break things like new mail notification and junk mail processing. | |||
There's no particular reason for the indexer to update local folders, since | |||
unlike IMAP, it doesn't really do anything interesting as far as the indexer is | |||
concerned. I guess you're doing it to make sure the .msf is up to date...which | |||
is a bit of an edge case, but I can see why you'd want to do that. But consider | |||
the imap case, where update folder will issue a select on the imap server and | |||
download new messages; does the indexer want to do that? As you've pointed out | |||
before, you'll need to sync with Emre's auto update stuff as well... | |||
You could actually just call GetDatabaseWithReparse for local mail folders. Or | |||
I could add a new method to nsIMsgFolder that does what you want for both kinds | |||
of folders. UpdateFolder is really meant more for opening folders in the UI, | |||
since it kinda assumes that's what's going on (hence the running of the junk | |||
mail controls). Or I could treat UpdateFolder with a null msg window as meaning | |||
it's not coming from the UI...</i> | |||
<b>(asuth [https://bugzilla.mozilla.org/show_bug.cgi?id=450494#c34 c34])</b> | |||
Right, I initially did this for local folders because sometimes in my testing | |||
(because of me, generally), the .msf files did not yet exist. I wanted to use | |||
GetDatabaseWithReparse, but wanted to use nsIMsgFolder and avoid a downcast. | |||
It seemed like a nice freebie that the IMAP state should be up-to-date to boot. | |||
(After all, what's the point of a global index if you have to click on things | |||
to get the results to be accurate...) | |||
Since Emre's code will be a given, I think I can rely on it to make sure the | |||
IMAP is sufficiently up-to-date. That just leaves avoiding dying when the msf | |||
doesn't exist yet. If that means an instanceof check and a call to | |||
GetDatabaseWithReparse, that works for me. | |||
<b>(asuth)</b> I did what what I said I would. We use GetDatabaseWithReparse. | |||
=== Standard8 === | === Standard8 === | ||