Closed Bug 646226 Opened 13 years ago Closed 13 years ago

Sent messages are not indexed anymore (with IMAP)

Categories

(MailNews Core :: Database, defect)

x86_64
All
defect
Not set
major

Tracking

(blocking-thunderbird5.0 needed)

RESOLVED FIXED
Thunderbird 5.0b1
Tracking Status
blocking-thunderbird5.0 --- needed

People

(Reporter: protz, Assigned: Bienvenu)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [datalossy])

Attachments

(2 files)

Now, with IMAP messages (I'm not using POP so I can't tell), sending a message now goes like this:
- a fake header is created while...
- we wait for the real header to come in and replace the fake header.

The fake header is wrongly indexed (see bug 646225), but the real header is absolutely, never, ever indexed. This can go on for hours. Sometimes, after restarting Thunderbird, it looks like indexing finally kicks in.
blocking-thunderbird5.0: --- → ?
If this is true, it sounds like we need to fix this.
blocking-thunderbird5.0: ? → needed
Severity: normal → major
Depends on: 646225
(I believe I'm seeing it locally).
(In reply to comment #1)
> If this is true, it sounds like we need to fix this.

indeed. is datalossy.   and hinders some testing of search issues. 

protz thinks this may fix bug 534449
Blocks: 574441, 534449
Keywords: regression
OS: Linux → All
Whiteboard: [datalossy]
Version: unspecified → Trunk
When my database is already "corrupted" due to this bug, will I have to delete and rebuild it once this bug is fixed?
(In reply to comment #4)
> When my database is already "corrupted" due to this bug, will I have to delete
> and rebuild it once this bug is fixed?

We can recover from the problem without blowing away the database by bumping the db schema by 1 and forcing a compaction pass on the sent folder(s) when we do so.
Assignee: bugmail → jonathan.protzenko
I think I'm seeing alternatively both this issue and 574441. It may be, if I'm lucky, that, just like Chuck Norris, I'll be able to kill two stones with one bird, and fix the two at the same time.

Just for reference on this particular issue, what I just saw is the following sequence:
- fake header gets added,
- displaying it with Thunderbird Conversations tells me that it hasn't been indexed, because getMessagesCollectionForHeaders returns nothing on that message (I have fancy debugging that tells me how we found the messages),
- afaict, no indexing happens in the meanwhile, because I have gloda dump enabled, and nothing appears on my console,
- msgKeyChanged notification (correct keys),
- I get the feeling that the code from msgKeyChanged assumes that the message has been indexed already, which is not the case, hence the 0 gloda-id and the failure propagating onwards.

/me keeps investigating
So I think my comment above was mostly right. Here's the bad sequence:
- fake headers is added,
- imap "sent" folder was open, so we replace the fake header with the right one immediately, hence issuing a msgKeyChanged notification
- msgKeyChanged from index_msg.js calls PendingCommitTracker.getGlodaState, which returns a gloda-id of 0,
- nonetheless, msgKeyChanged keeps going on and adds information in _keyChangedBatchInfo, although the information is *wrong* because Gloda is not aware of the existence of this message,
- then, we receive a msgClassified notification, which triggers a _reindexChangedMessages with aDirtying=false,
- _keyChangedBatchInfo says the message is not dirty (if I understood your comments properly, Andrew, this means that the message has been indexed by Gloda and is in a correct state), because that's what msgKeyChanged said,
- _keyChangedBatchInfo exits early with these two lines:

          if (!keyChangedInfo.isDirty)
            continue;

and the message is never indexed.

The patch I'm attaching basically says that msgKeyChanged should not try to do anything smart if Gloda hasn't indexed the fake header yet. It's a two-liner (four-liner including comments).

My plan is: let's land this, and see what happens. I don't think this will fix the other two issues:
- sometimes the fake header is not indexed (I've seen this: there's no msgClassified notification that we could act upon to index this message, probably need to clarify things with :bienvenu),
- https://bugzilla.mozilla.org/show_bug.cgi?id=646225
Attachment #529272 - Flags: review?(bugmail)
If my theory is correct, since we're not altering the msgDBHdr, restarting Thunderbird should be enough for an indexing pass to kick in and index all the sent messages that hadn't been indexed previously. At least that's what happened on my machine.
(In reply to comment #8)
> If my theory is correct, since we're not altering the msgDBHdr, restarting
> Thunderbird should be enough for an indexing pass to kick in and index all the
> sent messages that hadn't been indexed previously.

my sent messages go to inbox, and I too have found that restart gets them indexed. 

in a different (non-inbox/non-sent) folder however, selected messages don't get indexed.
Comment on attachment 529272 [details] [diff] [review]
Let's at least fix that, and see what happens afterwards checked-in 20a1948de3a9

Ugh, that was a really dumb oversight on my part :(
Thanks much for finding it!

The 0-check is not sufficient to test for a valid id, the usual check is:
  glodaId >= GLODA_FIRST_VALID_MESSAGE_ID

So, make this:
  if (glodaId < GLODA_FIRST_VALID_MESSAGE_ID)

Ideally we would like to augment the existing unit test for msgKeyChanged in gloda to make sure we don't handle messages that have not been indexed, but as you say, it seems good to land this asap.
Attachment #529272 - Flags: review?(bugmail) → review+
http://hg.mozilla.org/comm-central/rev/20a1948de3a9

This *may* be enough to close this issue, but this is only a speculative fix. In case the error seems to be fixed, I'll close that bug, and then I'll focus on bug 646225 for the rest of the work.

One way to test this is using Thunderbird Conversations. If you launch Thunderbird from a Terminal, it will output a lot of debug information. There's small codes in blue that appear at some point, and that tell you how we found each message in the conversation.
- GF = the message was indexed by gloda,
- MI = the message was NOT indexed by gloda.
- (there are other values but they're not relevant for that discussion).

If, while viewing a sent message with conversations, the terminal displays "MI" in blue, you know that your message has NOT been indexed. If, while viewing a sent message with conversations, the terminal shows "GF", then this message has been indexed.

It's possible that the first few messages in the conversation have been found by gloda, and the last one (if it's a message you sent) hasn't been indexed yet. In that case, the debug will look like: GF GF ... GF MI+G
Attachment #529272 - Attachment description: Let's at least fix that, and see what happens afterwards → Let's at least fix that, and see what happens afterwards checked-in 20a1948de3a9
Attached patch proposed fixSplinter Review
Protz, let me know if this fixes it for you...
Assignee: jonathan.protzenko → dbienvenu
Attachment #530415 - Flags: review?(jonathan.protzenko)
Comment on attachment 530415 [details] [diff] [review]
proposed fix

I just gave this patch a try, and Gloda is seeing msgsClassified for the fake header, which means it is indexed instantly (this is great). This also means asuth's code from bug 534449 will be actually used properly, because AFAICT, we were only going through this codepath with buggy assumptions before, or not going there at all!
Attachment #530415 - Flags: review?(jonathan.protzenko) → review+
fixed on trunk - http://hg.mozilla.org/comm-central/rev/ec4c84e6984a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: