465122 - Gloda: Does not close msf files on initial indexing

Status: RESOLVED FIXED

(MailNews Core :: Database, defect, P1)

Description

[Henry N.]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.6) Gecko/20070730 SUSE/2.0.0.6-25 Firefox/2.0.0.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1b2pre) Gecko/20081115 Shredder/3.0b1pre

After Gloda was indexing for the first time, some of msf are not closed.

The indexing was running about 30 Minutes.
After finish, 32 msf files still are opened. The files are in various subdirectories for example
"Mail/Local Folders/Inbox.msf",
"Mail/Local Folders/Project.sbd/Shredder.msf",
"Mail/Local Folders/Archiv.sbd/Send2006.msf",
"Mail/Local Folders/Archiv.sbd/Send2007.msf",
"News/news.arcor.de/de.comp.sys.novell.msf",
"News/news.XXXX.de/linux.kernel.msf",
"Mail/RSS-News+Weblogs/Trash.msf", and more.

"Mail/RSS-News+Weblogs/Trash.msf" is an empty folder (automaticly cleared on exit), with size of 1957 bytes in the file.


Reproducible: Always

Steps to Reproduce:
1.Starting a profile, where Gloda was never enabled
2.Be sure to have some "Local Folders", News Groups, RSS feeds (I have 23 on 2 servers)
3.Enable Gloda, restart Thunderbird
4.Wait to finish indexing (end of disk activity), and wait an extra minute
5.Run on Linux command line: 'lsof | grep ".msf"'

Actual Results:  
32 msf files opened


Expected Results:  
Only msf file opened, I'm currently viewing in folder pane. Normal are 6-10 files.


I'm not sure, it can depend on Bug #465113, or Bug #465117.

Comment 1

[David :Bienvenu]

this could be bad for some users, if they have a lot of folders and limited file handles, or large msf files and don't have infinite memory.  When gloda is finished with a folder, it should do folder.msgDatabase = null to clear the cached db, iirc.

Comment 2

[Wayne Mery (:wsmwk)]

I can't speak to the file handles, but in my last index of 103k imap+local messages on windows it showed ~100meg more memory than normal

Comment 3

[David :Bienvenu]

marking blocking b1

Comment 4

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

Attachment: v1 close nsIMsgFolders when we are done with them

The nsIMsgFolder actually implements setMsgDatabase explicitly, so folder.msgDatabase = null is right out.  We should fix the interface, but the breakage probability to benefit ratio is not worth it to fix immediately before a beta, in my mind.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 348932 [details] [diff] [review]
v1 close nsIMsgFolders when we are done with them

r=dmose; thanks for the patch

Comment 6

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

This fix is doing something bad to the gloda unit tests; I suspect it's just a test mechanism issue, but this should not land just yet.

Comment 7

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

Attachment: v2 close msf files when not indexing and after timeout, includes unit test

Per discussion with bienvenu, this uses a timer mechanism to detach the message databases after they are no longer in use.  In use means it is not being used for indexing and that it has been at least some arbitrary time (10 seconds currently) since a header has last been retrieved from that folder database.  Note that if a header is not retrieved, indexing will _immediately_ drop the database reference when it is done.  As such, the choice of constant ideally has no impact on indexing and memory use.

The potential 'gotcha' is that as things are indexed, they may be added to existing collections whose queries they match (which have not been made explicit).  (And until bug 464359 is shortly fixed, this means nearly all existing collections.)  Those collections may be attached to UI that wants to fetch the message's underlying header for streaming or other purposes, which will start the clock.  I'm not suggesting this is a major problem, I just want to make sure that this (non-obvious) factoid is available to reviewers so they can hopefully also decide that it's a non-issue.

This patch also includes some minor gloda testing framework changes so that it has more helpful markers when interesting things happen, and so that it fails outright (and noisily) on more callback/notification exceptions.  Otherwise we would tend to just hang and I would be confused as to why the test failed.

Comment 8

[David :Bienvenu]

Comment on attachment 349616 [details] [diff] [review]
v2 close msf files when not indexing and after timeout, includes unit test

this looks good in general, though I haven't applied it yet. 

One nit - incomplete thought here:

+    // we only need to keep going if there is at least one folder in the table
+    //  that

Comment 9

[David :Bienvenu]

Comment on attachment 349616 [details] [diff] [review]
v2 close msf files when not indexing and after timeout, includes unit test

r=me, if you fix the comment I pointed out before.

Usually we try to keep the db for inboxes around, on the theory that they're likely to be used frequently, but I think that's a minor issue and I don't think gloda needs to worry about it.

Comment 10

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

Attachment: v3 close msf files when not indexing and after timeout, includes unit test

resolved comment cliffhanger.  carrying forward r=bienvenu

Comment 11

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/7c7643e44d22

Comment 12

[Henry N.]

BuildID=20081125030435 (US nightly)

Mostly files are closed after finish the initial indexing.
But the last subfolder still leaved opened, for example
Profil.Henry/Mail/Local Folders/Archiv.sbd/Send2007.msf

This file was not touched by view pane in the time Gloda was running, and I was waiting now over 10 minutes.

In general, for me it is ok. I would just inform you about this small detail.