Right now, at least against Zimbra, if I archive a message, or reply to one, the message doesn't get reindexed by Gloda for a looong time if ever, except if I open up the correct Archive or Sent mail folder. My understanding is as follows, someone should correct it where I'm astray. 1) Archive & Reply both end up resulting in moves to IMAP/Archive/2010 or IMAP/Sent. 2) Autosync should index those quickly, but doesn't. 3) IMAP's IDLE should tell TB if those folders have new messages, but only if those folders are selected. In my case, I suspect that's rarely the case. 4) I'm not sure whether if we _did_ get an IDLE notification, whether that would result in a download & subsequent indexing. I don't understand enough about autosync to know when the current archive & sent folder should be indexed, but I wonder if it makes sense to explicitly schedule a resync of those any folder that we explictly move messages to, ideally with a small delay so that we avoid doing silly levels of autosync. As it stands, the conversation view is mostly broken, which feels a real shame.

I should emphasize that it's not the conversation view which is the real problem for 3.1 as much as the fact that global searches won't find these messages for a while.

several things - moving an imap message that has been auto synced also moves the offline copy into the target folder so it doesn't need to be autosynced, at least for the message bodies. But it does need to be selected so we can download the new headers. Autosync could be taught about this. Autosync doesn't listen for any sort of notifications, so there's probably a lot of low hanging fruit there. We know which folders have pending move/copies and how many messages there are - it's an attribute of nsIDBFolderInfo - imapTotalPendingMessages I can't remember if gloda listens for message move copy events. It could stay instantly up to date if it was willing to deal with the awkward two step process of a moved message having a temporary uid until we select the folder to find the real uid...

How is this different from bug 534449 ?

this covers that, but also dealing with messages that are moved to an other imap folder.

While trying to rewrap my head around the autosync code, I decided to fix this related bug - if you check non-inbox folders for new mail, autosync never thinks it should update those folders. This fixes that. It also contains a potential fix for not syncing unsubscribed folders, though that's not tested.

davida, here's an other patch (replacing the previous patch) to try. In addition to the STATUS fix in the previous patch, it adds folders that have messages added to them to the update q. Archive/Sent folders jump the queue, the rest are put at the end of the update q, except for the Trash, which is ignored. We could use the folder strategy to determine where to put messages in the queue. I also need to make non-autosync updates remove folders from the update q, and reset their last update time, so that we don't over update. With this patch, we will update folders that have messages moved into them on the next idle, which should be quite a bit sooner than before. Please let me know how it works for you.

(In reply to comment #2) > I can't remember if gloda listens for message move copy events. It could stay > instantly up to date if it was willing to deal with the awkward two step > process of a moved message having a temporary uid until we select the folder to > find the real uid... gloda listens for move/copy events. It is smart about the local case. The IMAP case is less awesome because of the whole UID problem. We basically do the following: 1) update our records so we know the messages are in the folder they got moved to but null the message key. 2) hope that someone goes into the folder and causes us to receive a msgsClassified notification when the headers officially show up with their UIDs. 3) ... someone goes into the folder and makes that happen ... 4) gloda does *not* fast-path the appearance of the message and performs a standard re-index. It's not as bad as if we had never seen the message before because we will avoid updating the fulltext index and may avoid changing some index rows because of our delta logic. I assume the ideal case would be something like: a) gloda hears the messages got moved to an IMAP folder b) gloda/a friend forces a prompt UPDATE on the folder c) gloda gets told about the new UIDs/messageKeys for the message and is able to tell that only the UID changed. It sounds like this patch improves the state of 'b' although still requiring an idle. For 'c', my naive optimization would be to keep feeding off of msgsClassified and just to perform a straight-up retrieval on each message given its gloda-id and verify that the message key is null. If we see that, we can perform an update without streaming the message or re-running any attribute providers. bienvenu, is there anything more clever than msgsClassified that it would be worth considering?

bienvenu: putting the folder at the top of the queue is good, but I don't think it's really good enough. I find often that I end up with the following: - reply to a message, and often quite soon after I send it, - look at the message again and for whatever reason want to find my reply Or I'll archive some messages in a thread but keep the latest one, and then decide i want some context. In either case, if 'find in conversation' or a gloda find isn't reliable, I don't use them for built-up lack of trust in gloda.

davida/asuth - I think the only way to get instantaneous updates is to take the imap server out of the equation and use the fake offline header until we've updated the folder. When we do update the folder, we'll need to change uid of the message from the fake one to the real one, and we'll need some sort of notification like OnKeyChanged(nsIMsgDBHdr, oldKey, newKey). For move/copies, this means keeping the fake offline header around even after the offline operation is played back. For fcc, it means creating a fake header and perhaps fcc'ing automatically to the offline store. All that's too scary for 3.1, but should be doable for 3.2. This patch, on the other hand, is probably OK for 3.1 (and the first STATUS patch SHOULD be in 3.1)

Comment on attachment 441951 [details] [diff] [review] fix issue with biffing non-inbox folders working on a unit test for this now.

I've spent the last few days trying to write a unit test that fails w/o the patch and succeeds with it. This patch in this test does not meet that criteria. Autosync is rather difficult to control, which makes unit tests hard. One thing to note is that because we maintain the offline bodies when copying messages between imap folders, the update of the target folder is all we need to do for gloda to know that it can index the message in the target folder. I'm not sure that gloda knows that, but I'm also not sure that we've set the offline flag on the new message before we tell gloda about it.

(In reply to comment #11) > I've spent the last few days trying to write a unit test that fails w/o the > patch and succeeds with it. This patch in this test does not meet that > criteria. Yeah, seems hard and more trouble than it's worth. Much better to just be confident autosync is playing by the new rules. It sounds like the worst problem is newly sent messages placed in the sent folder not getting indexed in a timely fashion. Do we think this patch will resolve that problem and do so without requiring an idle cycle? If not, we should probably spin off another bug for that.

(In reply to comment #12) > > It sounds like the worst problem is newly sent messages placed in the sent > folder not getting indexed in a timely fashion. Do we think this patch will > resolve that problem and do so without requiring an idle cycle? If not, we > should probably spin off another bug for that. That needs a different bug. I think what needs to happen is that we essentially fcc the offline store if the Sent folder is configured for offline use. Then, on servers that support UIDPLUS (which is pretty common), we'll be much more timely. I'll file a bug on that.

This patch does several things: 1. If the "check this folder for new messages" biff STATUS tells us there are new messages, put the folder in front of the update q. 2. Exclude orphan folders from autosync 3. Put folders that have messages moved/copied into them in front of the update q. 4. Add an autosync unit test that we can build on and use to test future autosync improvements. 5. Clean up a lot of whitespace issues.

This patch does not do what I described in bug 562115#c9 - that'll be in my next round of autosync improvments.

Oh, and you'll need the patch in bug 580108 in order for the unit test in this patch to fail without the core changes.

Comment on attachment 458777 [details] [diff] [review] first round of fixes, with unit test > nsresult nsAutoSyncState::OnNewHeaderFetchCompleted(const nsTArray<nsMsgKey> &aMsgKeyList) > { >- return PlaceIntoDownloadQ(aMsgKeyList); >+ SetLastUpdateTime(PR_Now()); >+ if (!aMsgKeyList.IsEmpty()) >+ return PlaceIntoDownloadQ(aMsgKeyList); >+} Not all code paths return a value (does it even need to?). r/sr=Standard8 with that fixed.

fix checked in. I've seen the test fail once, in about 20 attempts, so it wouldn't shock me to see the test fail sometimes...

this breaks tests, so I've backed this out.

I've pushed this to try to see if we can replicate the mozmill failures. My suspicion is that it may not, but it'll at least give us an idea.

(In reply to comment #20) > I've pushed this to try to see if we can replicate the mozmill failures. My > suspicion is that it may not, but it'll at least give us an idea. I've not been able to reproduce this on try or locally on my Mac. My next thought is to try locally on one of the tinderboxes which I won't have time to do until next week.

I wrote this patch a while ago, so I don't remember anything that changes initialization of the service in a way that might break tests, but I'll go through the patch some more...

this moves the initialization of the various services out of the autosync manager constructor into the init method so we can get real errors if something fails.

The imap service initializes the autosync manager in its constructor, which was a bit surprising to me.

To complete the story, we relanded this with the following additional fixes: http://hg.mozilla.org/comm-central/rev/c9b340b230e8 http://hg.mozilla.org/comm-central/rev/04e15589b99f - fix string array out-of-bounds access. http://hg.mozilla.org/comm-central/rev/521c6f3300d7 - add a comment about the strings http://hg.mozilla.org/comm-central/rev/960a02225e3d - create a mock idle service so that the test doesn't randomly hang on windows. It appears that Mac still has a hang on shutdown for the unit test, which we'll be dealing with in a separate bug.

(In reply to comment #9) > the only way to get instantaneous updates is to take the > imap server out of the equation and use the fake offline header until we've > updated the folder. When we do update the folder, we'll need to change uid of > the message from the fake one to the real one, and we'll need some sort of > notification like OnKeyChanged(nsIMsgDBHdr, oldKey, newKey). For move/copies, > this means keeping the fake offline header around even after the offline > operation is played back. For message moves, e.g., a delete to trash, or archive, we leave the fake header in the dest folder until the dest folder is updated, so there's a very small window when we don't actually have the message and body (i.e., between the time we start the update and the time we finish it). To gloda, the fake header looks like a real message, because the key we use is actually just the folder highwater mark + 1, so gloda should be able to keep up to date today. It might be the case that gloda's not listening for the particular notifications that get sent, or it may just be assuming that it can't get at the info it needs in the imap move/copy case. For the fcc case, I believe we're already fcc'ing the offline store, though I'm not sure what notifications are sent.