912216 - (condstore-default) disable CONDSTORE support

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

As discussed in tb-planning[1], CONDSTORE support has recently been rolled out to all GMail users who get their email through IMAP. We've been getting periodic reports that some GMail users are not receiving their email in a timely fashion, and that disabling CONDSTORE fixes the issue.

We might want to temporarily disable CONDSTORE for GMail until we understand what is actually going on here.

This is also a thing we might want to roll into the 24 release, as a convenient bonus. Failing that, it would make sense in a point release.

Cc'ing folks who might be interested in this topic.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

[1]: https://groups.google.com/forum/#!topic/tb-planning/2ZMdeUbzV0A

Comment 2

[Mike Conley (:mconley) (:⚙️)]

From protz:

"The problem is only really visible if you use labels a lot, that is, if 
you expect mails in folders other than inbox to be fetched actively. 
This may not be the case for the general GetSfn population."

Comment 3

[al_9x]

Bug 885220

Comment 4

[WADA:World Anti-bad-Duping Agency]

(In reply to Mike Conley (:mconley) from comment #0)
> We've been getting periodic reports that some GMail users are not receiving their email
> in a timely fashion, and that disabling CONDSTORE fixes the issue.
(In reply to Mike Conley (:mconley) from comment #2)
> From protz:
> "The problem is only really visible if you use labels a lot, that is,
> if you expect mails in folders other than inbox to be fetched actively. 

If so, this is mainly relivant to "UID fetch 1:* (FLAGS) (CHANGEDSINCE 117803)" after "SELECT mbox" by folder open when CONDSTORE is used and CONDSTORE support of Tb is enabled.
If Inbox, when first Inbox open just after restart of Tb(by check new message on startup), Tb issues "UID fetch 1:* (FLAGS)" (without CHANGEDSINCE) even when CONDSTORE is used and CONDSTORE support of Tb is enabled. This is trick of "this bug is not seen on Inbox".
See 885220 comment #66, bug 885220 comment #75, and IMAP log file attached to bug 885220.

As I wrote in bug 885220 comment #86, I also believe "disable CONDSTORE support by default" is best practice especially when Gmail IMAP account.

Comment 6

[WADA:World Anti-bad-Duping Agency]

FYI. 
How to disable CONDSTORE support by Tb manually.
  Disable CONDSTORE support by Tb for all IMAP accounts.
    mail.server.default.use_condstore = false
  Disable CONDSTORE support by Tb for each IMAP accountFYI. independently.
    mail.server.server#.use_condstore = false

Comment 7

[WADA:World Anti-bad-Duping Agency]

Why default of mail.server.default.use_condstore = false is not set in mail.js yet?
Because Tb user who itentionally sets mail.server.server#.use_condstore=true is rare, I think "mail.server.default.use_condstore=false in mail.js" is sufficient to protect almost all Tb users who use IMAP server with CONDSTORE extension enabled.

Comment 8

[Wayne Mery (:wsmwk)]

comments in bug 885220 indicates there are ongoing gmail issues, but we seem indecisive in this bug.

small poll, though I'm not sure it would be adequately representative of a larger population of users:
- do you see this problem with gmail?  
- and have you seen support requests about this issue?   

myself - I've had condstore disabled for a long time, so I'm not in a position to judge whether things have improved with gmail.

Comment 9

[David Lechner (:dlech)]

I don't use gmail as my main email anymore, so I can't offer any useful feedback here.

Comment 10

[Patrick Cloke [:clokep]]

I have seen this issue with my my gmail account(s), particularly with different folders I'll frequently need to click on a folder to have it update and download the mail. I set 'mail.server.default.use_condstore' to false last night and have noticed a different in how quickly I receive mail: I seem to get things individually instead of in huge batches and sub-folders will update with no user intervention.

Comment 11

[James Rome]

I do not remember this. Nor do I know how to disable CONDSTORE.

Comment 12

[rsx11m]

Judging from http://forums.mozillazine.org/viewtopic.php?f=39&t=2870747 this still seems to be an issue in current releases (I've had it disabled on all installations since the issue came up).

Comment 13

[rocketraman]

Is there a separate bug for CONDSTORE issues for non-gmail servers? I use cyrus imapd 2.4, and CONDSTORE is returned as a capability.

With an imap account on this server, I definitely find that Thunderbird 31.3.0 updates read/deleted status changes more consistently with CONDSTORE disabled in Thunderbird.

Comment 14

[rsx11m]

Attachment: Just flip the default for now

Let's go for it. If I remember correctly, interoperability issues with CONDSTORE were reported at least for three common IMAP servers (including GMail). This is the 38.0 cycle, i.e., whatever is done now will stick with Thunderbird for about another year, so this would be a good opportunity to take preliminary action.

This patch simply flips the default, thus it's still possible to re-enable the feature globally or for specific servers if they are not affected by bug 885220 or related. Judging from bienvenu's comment in the line before the default is set, he seemed to have some doubts in the state of implementations on either client or server sides as well.

Comment 15

[[e-mail]]

Comment on attachment 8549951 [details] [diff] [review]
Just flip the default for now

Sorry, I can't make this call.

Comment 16

[Magnus Melin [:mkmelin]]

Since we just have reports of GMail problems, probably shouldn't disable it for all.

Comment 17

[rsx11m]

There is bug 727790 stating that CONDSTORE-triggered issues occur with a Dovecot server, I can't find the third report that I had in mind (it may have been in a GetSatisfaction thread which since got wiped out). On the other hand, I see the argument that if only a handful of servers are affected, disabling it globally may be excessive (the question being how many servers actually use that feature - it may not be a problem there because it's simply not advertised, thus wouldn't make a difference either way).

Comment 18

[rocketraman]

(In reply to Magnus Melin from comment #16)
> Since we just have reports of GMail problems, probably shouldn't disable it
> for all.

See comment #13: this affects cyrus imapd as well.

Comment 19

[Kent James (:rkent)]

CONDSTORE is supposed to prevent issues when multiple clients access the same mailbox. What are some possible downsides of disabling it?

"The Conditional Store facility provides a protected update mechanism for message state information that can detect and resolve conflicts between multiple writing mail clients."

Comment 20

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

(In reply to Kent James (:rkent) from comment #19)
> CONDSTORE is supposed to prevent issues when multiple clients access the
> same mailbox. What are some possible downsides of disabling it?

The main advantage is more efficient flag synchronization.

Without CONDSTORE, Thunderbird does something like this:
  A1 UID FETCH 1:* (FLAGS)
and gets *all* of the flags for all of the messages in the folder.  (This also allows for deletion inference.)

With CONDSTORE, Thunderbird can do this:
  A2 UID FETCH 1:* (FLAGS) (CHANGEDSINCE 12345)
and gets *only* the flags that had changed since the given MODSEQ.

Note that CONDSTORE support without QRESYNC support can still require large-scale flag fetches to infer deletion.  QRESYNC added the idea of saying what messages got expunged AKA VANISHED.  (See https://tools.ietf.org/html/rfc7162#section-3.2.5.1 and https://tools.ietf.org/html/rfc7162#section-3.2.6).

Gmail potentially has more issues than other servers because for a long time they would only generate IDLE notifications in a folder when a messages was added or deleted, but *not when flags were changed*.  I think gmail might have been the only server out there that would generate this subset of expected notifications.  On October 6th, 2014, Gmail rolled out better IDLE support, see http://mailman13.u.washington.edu/pipermail/imap-protocol/2014-October/002350.html.  However, they seem to have almost immediately rolled it back and :bsmedberg has indicated it still seems to not have been re-enabled again.  See http://mailman13.u.washington.edu/pipermail/imap-protocol/2014-October/002358.html.

I am not up on Thunderbird's CONDSTORE implementation or exactly what the manifestation of the problems are.  The most likely explanation would be that Thunderbird somehow ends up missing specific deltas.  And once you update your MODSEQ, you aren't going to get a replay of the flags from earlier.  Disabling CONDSTORE will of course avoid that problem at the cost of greater bandwidth usage.

> "The Conditional Store facility provides a protected update mechanism for
> message state information that can detect and resolve conflicts between
> multiple writing mail clients."

This is https://tools.ietf.org/html/rfc7162#section-3.1.3 which mainly lets flag changes be predicated on the message not having had other flag changes since what the user saw.  (So the example of: user has 2 devices, A and B, user makes a flag change on device B, but then B is offline for a long time, the user makes some other change on device A which is online, so when B actually goes to make the flag change, it is acting on very stale data and it can know to not clobber the user's changes on device A.)

I don't know if Thunderbird actually supports this at all.

Comment 21

[Kent James (:rkent)]

Based on Andrew's comments, it seems the case is pretty strong for disabling for GMail. I don't see as strong a case for other servers.

I would hate to disable useful functionality for non-GMail IMAP servers based primarily on issues with GMail. We need a stronger case than simply comment 13 to do that.

Comment 22

[rocketraman]

(In reply to Kent James (:rkent) from comment #21)
> We need a stronger case than simply comment 13 to do that.

What would form such a case? Some other examples of non-gmail people reporting improvements in behavior with CONDSTORE disabled:

- Dovecot : bug #885220 comment 95
- Zimbra : bug #885220 comment 96

Comment 23

[rocketraman]

I was able to create a relatively simple reproduction recipe for a problem with Thunderbird CONDSTORE and cyrus-imapd. See bug #1123094. I don't know if other servers would also demonstrate this or similar issues.

Comment 24

[WADA:World Anti-bad-Duping Agency]

(In reply to Kent James (:rkent) from comment #21)
> Based on Andrew's comments, it seems the case is pretty strong for disabling for GMail.
>  I don't see as strong a case for other servers.

Why can problem like bug 885220, Bug 693204 be Gmail IMAP specific problem?
Why can such bugs be ignored if non Gmail IMAP server?

If server supports CONDSTORE and Tb's CONDSTORE support is enabled, bug 885220 occurs, and if server doesn't notify "flag state change" via iDLE, Bug 693204 occurs. Bug 1123094 is report on non Gmail IMAP but Bug 1123094 should occur with Gmail IMAP if "UID fetch 1:* (FLAGS) (CHANGEDSINCE nnnn)" is used by Tb after "SELECT Inbox". Bug 517461 was irrelevant to Gmail IMAP because Gmail didn't enable CONDSTORE when the bug was found, but the bug should have occurred if Gmail had enabled CONDSTORE.

A reason why this bug was opened for Gmail IMAP is;
- Number of Gmail IMAP users who uses Thunderbird is large.
- Because Gmail IMAP user can know about new mails in Inbox by new mails in [Gmail]/All Mail,
  Gmail IMAP+Thunerbird user can easily be aware of "new mail alert of new mails in Inbox was not somehow shown by Tb".
- Loss of user by "no new mail alert" is far larger than gain of user by current CONDSTORE support use.

Comment 25

[WADA:World Anti-bad-Duping Agency]

(In reply to Kent James (:rkent) from comment #19)
> CONDSTORE is supposed to prevent issues when multiple clients access the same mailbox. 
> "The Conditional Store facility provides a protected update mechanism for message state information
> that can detect and resolve conflicts between multiple writing mail clients."

If current Tb's CONDSTORe support is used, it surely produces big issue when multiple clients access the same mailbox pretty easily.
See bug 1123094, please.

By the way, why "opening this bug" can not be sufficient for getting pref("mail.server.default.use_condstore", false); in mainews.js to avoid unwanted bug opens and to reduce victims by bugs?

Comment 26

[WADA:World Anti-bad-Duping Agency]

As known by bug 885220 commen #88 and problem analyss by bug opener of bug 885220, "why bug 885220 occurs" was;
1. When Tb's CONDSTORE support is used, Bug 517461 occurred.
     Then, *Unread count* is obtained from "UNSEEN n" response only if CONDSTORE support is used.
     If new mail is detected via IDLE, *Unread count* is not modified after fix of Bug 517461, if CONDSTORE support is used.
2. IDLE command use is enabled by default. Defult of max cached connections = 5.
    So, if Inbox,
    - Inbox is always selected at a cahed connection and is IDLE state almost all time,
    - Status commaand, which returns "UNSEEN", can not be used by Biff because already selected at a connection.
       and, "uid fetch NextUID:* flags (CHANGEDSINCE Largest_MODSEQ_Tb_Knows)" is used by Tb.
    - Probability of "new mail is detected by uid fetch NextUID:* flags" is far smaller than
       probability of "new mail is detected via IDLE.
    Because almost all mails are detected via IDLE, there is no chance of "*Unread count* update by UNSEEN n response".
3. Because new mial alert of Biff is based on the *Unread count*, new mail alert is not shown.

Please note that no phenomenon due to "particularity of Gmail IMAP" is involved in above issue.

Comment 27

[WADA:World Anti-bad-Duping Agency]

(In reply to Magnus Melin from comment #16)
> Since we just have reports of GMail problems, probably shouldn't disable it for all.

What is evidence of that "Almost all known CONDSTORE relevant Tb's issues" is caused  by "particularity of Gmail IMAP"?
Why such "Almost all known CONDSTORE relevant Tb's issues" can be ignored if "non Gmail IMAP server who supports CONDSTORE"?

Comment 28

[Kent James (:rkent)]

So as I understand it, WADA and rocketraman (and I assume rsx11m) are supporting disabling CONDSTORE per comment 14, neil is neutral.

In response to comment 22 "What would form such a case?" the ongoing discussions are making the case. Magnus, Joshua, what do you think? WADA has been tracking this, and I'm influenced by his support to proceed with the patch, but I don't feel I know enough to make the call either.

Comment 29

[rsx11m]

(In reply to Kent James (:rkent) from comment #28)
> So as I understand it, WADA and rocketraman (and I assume rsx11m) are
> supporting disabling CONDSTORE per comment 14, neil is neutral.

Yes, I do. As long as major IMAP servers don't work correctly with it enabled rather than being specific to a single server (see WADA's analysis), it shouldn't be switched on by default.

Comment 30

[WADA:World Anti-bad-Duping Agency]

(In reply to Kent James (:rkent) from comment #21)
> I would hate to disable useful functionality for non-GMail IMAP servers based primarily on issues with GMail.

I rather hated disabling CONDSTORE support merely by "new mail alert is not always shown", because I thought root cause of bug 885220 was in Biff code, even if problem was triggered by CONDSTORE support use. Why I wanted "disabling CONDSTORE" was simply "it's simplest/easiest way".
However, after I knew problem of bug 1123094, I believe that current CONDSTORE support shouldn't be enabled.

History is perhaps as follows.
1. When Tb's CONDSTORE support was used, bug 1123094 occurred.
    Because Tb shows unread count only at folder pane, users couldn't be aware of "total number of mails in folder is wrong",
    "total size of folder is wrong", "already expunged mail is still shown at Thread pane".
    If mail of "expunged but Tb couldn't be aware of expunge" was Unread mail, Unread count shown in Tb is different from count shown by other client.
    So, users reported "wrong unread mail count" only. This is Bug 517461.
2. Becuse of above characteristics of problem, sufficient data for problem analysis coudn't be provided.
    Almost all reports was "Unread count is wrong" only.
3. Because correct Unread count is obtained by "UNSEEN" response,
    code was changed to "set Unread count from  UNSEEN response if CONDSTORE is used" by Bug 517461.
4. Many bugs were opened for "new mail alert is not consistently shown", but no one could find culprit.
5. Gmail IMAP started to support CONDSTORE.
    Because  Gmail shows a new mail in both Inbox and [Gmail]/All Maill, 
    user can easily be aware of "new mail surely exists in Inbox but new mail alert was not shown" by new mail in [Gmail]/All Mail.
    And, phenomenon is sufficiently inconsistent.
    -   if new mail in [Gmail]/All Mail is detected by new mail check of Biff, new mail alert for new mail in Inbox is shown.
    -   if new mail in [Gmail]/All Mail is detected by IDLE, new mail alert for new mail in Inbox is not shown too.
6. By bug 885220, detail of phenomenon of "new mail alert is not shown if new mail is detected by IDLE" was known.
7. After it, bug 1123094 was reported.
    If IDLE is not used, EXPUNGEd mail is not removed from Tb if \Deleted flag of the mail is not detected by Tb.

I think following is needed.
- Resolve bug 1123094, with enhancement of QRESYNC support, with utilizing QRESYNC. 
- Backout change by Bug 517461. (solution of bug 885220. no need of change by Bug 517461 after resolving bug 1123094)
- Rewrite Tb's CONDSTORE support appropriately.
- Enhance Tb's CONDSTORE support. Tb's CONDSTORE support should be solution of Bug 693204.
   "uid fetch 1:* (CHANGEDSINCE ...)" is always needed for this purpose.
I believe Tb's current CONDSTORE support should be disabled until above enhancements will be made.

Comment 31

[WADA:World Anti-bad-Duping Agency]

Correction on phenomenon of Bug 517461.
After testing bug 1123094, I read Bug 517461 again. IDLE was used and worked well, so it was not bug 1123094.
   Bug 517461 == perhaps "problem of Bug 693204" + "current CONDSTORE support can not be solution of Bug 693204".

Mechanism of Bug 517461 is perhaps:
  1. UD=A(MODSEQ=a), UID=B(MODSEQ=a+1) is known. HighestUID=B, NextUID=B+1
  2. new mail check by Biff issues "uid fetch NextUID(b+1):* flags"
      If flag change occurrs on UID=a(MODSEQ=b+2) and UID=B(MODSEQ=b+3), following is returned.
          FETCH (UID B MODSEQ (b+3 FLAGS ($label1 $label3 NonJunk \Seen))
      Because request is for "uid B+1:*", change on UID=A(MODSEQ=b+2) is not returned.
      => Highest MODSEQ Tb knows == b+2
 3. When Mbox is explicitly opened by folder click, following is issued.
         "uid fetch 1:* flags (CHANGEDSINCE b+2)
      Because of (CHANGEDSINCE b+2), flag change of MODSEQ=b+1 on UId=A is not notified to Tb from server.
      => If "flag change of MODSEQ=b+1 on UId=A" contains \Seen flag change, mismtch of Red/Unread count occurs.
      If checked by aadding/removing Tag, phenomenon can easily be observed by color change.
If CONDSTORE support is disabled, "uid fetch 1:* flas"(without CHANGEDSINSE) is used, so flag change of all mails is detected by open of Mbox.
This is trick of "no problem when CONDSTORE support is disabled in Bug 517461".

To know flag change  by utilizing CONDSTORE, "uid fetch 1:* flags (CHANGEDSINCE known_modseq)" is mandatory.
"uid fetch NextUID(b+1):* flags"(without CHANGEDSINCE) is requested by Biff.
So, problem can be called "Biff code doesn't work correctly if CONDSTORE suppprt is enabled" or "CONDSTORE support failed to modify Biff code".

Comment 32

[Nikolay Shopik]

Just for reference bug 491445 is QRSYNC request

Comment 33

[WADA:World Anti-bad-Duping Agency]

FYI.
I've opened Bug 1123617 for issue of comment #31.

Comment 34

[WADA:World Anti-bad-Duping Agency]

I think following is sufficient.
(1) mail.server.default.use_condstore=false in mainews.js ASAP
(2) Before fix of bug 491445 (QERESYNC)
(2-1) Fix  bug 1123094
         After "SELECT Mbox (CONDSTORE)" at any connection, issue "uid fetch 1:* Flags" without CHANGEDSINCE
         (to avoid loss of EXPUNGED state. this is reason why QRESYNC is needed)
(2-2) Fix bug 1123617 
         At any cached connection, when Mbox is already selected, Biff requests "uid fetch 1:* Flags (CHANGEDSINCE KnownLrgestMDSEQ)"
         (to avoid loss of flag state change, to know flag state change of already fetched mails)
         By this change, bug 693204 is automaticlly resolved.
(2-3) Backout change of bug 517461            
          bug 885220 is resolved.
          Because problem of bug 517461 is resolved by fix of  bug 1123617, patch by bug 517461  is not needed any more.
(3) When (2) completed,  mail.server.default.use_condstore=true in mainews.js
(4) After fix of  bug 491445 (QERESYNC)
      Rewrite code appropriately

Comment 35

[rsx11m]

WADA, if you want to be more specific with the title you'll have to add SeaMonkey too, given that it's equally affected by all related bugs. Also, I'm making them all block this one, thus we get notifications when they are fixed (if at all) and it may be safe to enable the feature by default again (where this bug here would be redundant if all of those would be fixed in time for 38.0, which I doubt will happen though).

Comment 36

[Joshua Cranmer [:jcranmer]]

(In reply to Kent James (:rkent) from comment #28)
> So as I understand it, WADA and rocketraman (and I assume rsx11m) are
> supporting disabling CONDSTORE per comment 14, neil is neutral.
> 
> In response to comment 22 "What would form such a case?" the ongoing
> discussions are making the case. Magnus, Joshua, what do you think? WADA has
> been tracking this, and I'm influenced by his support to proceed with the
> patch, but I don't feel I know enough to make the call either.

I am not an expert in the IMAP backend. Having asked around about the quality of server-side support, it sounds like the issues are in our client as opposed to the server for the most part. It seems appropriate, if regrettable, that we should disable CONDSTORE for the time being.

Comment 37

[rsx11m]

Thanks for the r+ and to everybody weighing in, push for comm-central please.

Comment 38

[WADA:World Anti-bad-Duping Agency]

Quick summary of currently known issues.
(1) bug 1123094 : with CONDSTORE, if  \Deleted flag/EXPUNGE response is not passed from server,
                               (if cached connection for Mbox is stolen by other Mbox, tis situation occurs)
                               msgDBHdr of the deleted mail won't be removed.
                               "uid fetch 1:* Flags without CHANGEDSINCE" should be used after SELECT even when CONDSTORE is used.
                               IIUC, this is a reason why QRESYNC is needed.
(2) bug 1124569 : With CONDSTORE,when \Deleted flag is not notified from server,
                               (when IDLE is not used, this situation occurs. even if IDLE is used, many servers don't notify it via IDLE)
                                "* n EXPUNGE response to noop upon next new mail check by Biff" is not correctly processed,
                               then msgDBHdr of the deleted mail won't be removed. Pure bug of CONDSTORE support .
(3) bug   693204 : Without CONDSTORE, new mail check by Biff is done by "uid fetch NextUID:* Flags" without CHANGEDSINCE.
                               So, if IDLE is not used, or if imap serer doesn't notify flag change via IDLE, flag change of already fetched mail can not be known,
                               until next full sync(uid fetch 1:* Flags  without CHANGEDSINCE, because CONDSTORE is not used) .
(4) bug 1124569 : With CONDSTORE, new mail check by Biff is done by "uid fetch NextUID:* Flags (CHANGEDSINCE know_modseq).
                               And, first sync after SELECT is partial sync(uid fetch 1:* Flags (CHANGEDSINCE known_modseq).
                               So, if IDLE is not used, or if imap serer doesn't notify flag change via IDLE,
                               flag change of already fetched mail can not be known until next full sync(uid fetch 1:* Flags without CHANGEDSINCE).
                               Because CONDSTORE is usable, new mail check by Biff should be "uid fetch 1:* Flags (CHANGEDSINCE known_modseq)",
                               in order to resolve problem like bug 693204 by utilizing useful CONDSTORE.
                               IIUC, one of major purposes of CONDSTORE is this kind of imap server access.
(5) bug  885220  : Due to change by bug 517461,
                               if CONDSTORE is used, and if rew mail is detected via IDLE, new mail alert by biff is not shown.
                               Because cause of bug 517461(wrong Unread count) is found to be above bugs, change by bug 517461 should be backed out.

Comment 39

[WADA:World Anti-bad-Duping Agency]

FYI.
Quick conlusion.
Simplest solution is:
    Until QRESYNC is supportrd by Tb, even when CONDSTORE is usable, do full-resync always after SELECT Mox at a cached connection.
              SELECT Mbox (CONDSTORE) / uid fetch 1:* Flags (CHANGEDSINCE known_modseq)
        => SELECT Mbox (CONDSTORE) / uid fetch 1:* Flags without CHANGEDSINCE
    When CONDSTORE is usable, do partial-resync upon new mail check by Biff instead of "checking NextUID".
        uid fetch NextUID:* Flags  => uid fetch 1:* Flags (CHANGEDSINCE known_modseq)
Fault in current CONDSTORE support of Tb is :
   Even though QRESYNC is not usable yet in Tb,
   and even though "EXPUNGE notification" is message number basis instead of UID basis,
   and even though IDLE is not always usable/effective in any environment,
   and even though connection can be lost while IDLE status when IDLE is used/effective,
   Tb's CONDSTORE support used partilal-resync after SELECT Mbox.
   Unless QRESYNC is supproted, CONDSTORE should have been used only for new mail check by Biff(periodical check by "every NN minutes" setting).

Comment 40

[WADA:World Anti-bad-Duping Agency]

FYI.
I've opened Bug 1124924.
Bug 1124924 - If CONDSTORE is usable, use "uid fetch 1:* flags (CHANGDEDSINCE modseq)" for new mail check by Biff instead of "uid fetch NextUID:* flags", in order to resolve problem like Bug 693204, Bug 517461
Note:
Following will resolve all issues discovered by this bug.
      (a) "SELECT Mbox (CONDSTORE) + uid fetch 1:* flags without CHANGEDSiNCE" by "Check new messages upon startup" and  by "folder click"
  + (b)  "uid fetch 1:* flags (CHANGEDSiNCE modseq)" upon new mail check of Biff by "Check new messages every NN minutes"
(a) is solution for "EXPUNGE notification is message number basis instead of UID basis".
(b) is solution for "IDLE is not always used", "some imap servers don't notify flag change via IDLE", "connection can be lost while IDLE state".

Comment 41

[WADA:World Anti-bad-Duping Agency]

(In reply to Andrew Sutherland [:asuth] from comment #20)
> Note that CONDSTORE support without QRESYNC support can still require large-scale flag fetches to infer deletion.
> QRESYNC added the idea of saying what messages got expunged AKA VANISHED.
>  (See https://tools.ietf.org/html/rfc7162#section-3.2.5.1 and https://tools.ietf.org/html/rfc7162#section-3.2.6).

Thanks a lot for your explanation. 
Without it, I couldn't understand Bug 1123094 opened by [e-mail]. Without it, I couldn't re-read  Bug 517461 and understand what had happened in the past. Without it, I coouldn't open Bug 1124569 for Bug 693204 even though I thought that "uid fetch 1:* flags CHANGEDSNCE upon new mail check" may be a solution of Bug 693204.

A question.
Is there any enhancement in "* MsgNo EXPUNGE" response by extension such as QRESYNC?

As seen in imap log attached to  Bug 1124569,  "* MsgNo EXPUNGE" is returned to "noop" command, in addition to unsol response to IDLE. IIRC,  "* MsgNo EXPUNGE" is returned to check command too.
If this "* MsgNo EXPUNGE" response is enhanced to format like "* MsgNo EXPUNGE (UID xxx)" or "* MsgNo EXPUNGE (VANISHED xxx)", there is no need to issue additional commands when "* MsgNo EXPUNGE" response is returned, as far as imap server respects RFC and supports the extension correctly.

By the way, as for "flag state change notification via IDLE", I don't know actual imap log for it. I didn't see log of "flag state change notification comes from server via IDLE" in any map log attached to bugs in B.M.O which I read in the past. IIUC, Gmail IMAP is never only one imap server who doesn't notify flag change via IDLE.
Bug 693204 is never Gmail IMAP specific issue, even though majority of dups of Bug 693204 is report for Gmail IMAP.
Bug 517461 is not opened for Gmail IMAP, and "flag state change notification from server via IDLE" is not seen in attached imap log. IDLE is surely used in this case, and needless to say, Read/Unread==With \Seen flag/Without \Seen flag, and Tb uses "uid fetch NextUID:* Flags" upon new mail check by Biff regardless of CONDSTORE use. So, phenomenon of Bug 693204 is involved in Bug 517461 which is complaint of "Unread mail count is wrong", although other phenomenon than Bug 693204 is also surely involved in that bug.

Comment 42

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

(In reply to WADA from comment #41)
> Thanks a lot for your explanation.

Glad it helped!  I've been skimming the messages going by and it looks very much like you have identified the bugs in Thunderbird's behaviour!

> Is there any enhancement in "* MsgNo EXPUNGE" response by extension such as
> QRESYNC?

Yes.  Once "ENABLE QRESYNC" is sent to a QRESYNC-enabled server, instead of generating EXPUNGE responses it should only generate VANISHED responses for the duration of the connection.  VANISHED responses are always in reference to UIDs.  (Note that this does not hold for folders which are NOMODSEQ.)

There are more details at https://tools.ietf.org/html/rfc7162#section-3.2.10

https://tools.ietf.org/html/rfc7162#section-6 also has the following example which confirms my understanding (I hope! :)
   C: A152 NOOP
   S: * VANISHED 8
   S: A153 OK [HIGHESTMODSEQ 104] done


In terms of IDLE notifications, I'm afraid I don't have a lot of insight or experience with what various servers actually do.  The Firefox OS email app does not yet do anything with IDLE/NOOP/unsolicited FETCH notifications, so we haven't run into problems there.  Over the course of the next few months we'll be doing some prototyping with gmail and I may know more then; I'll be sure to send a message to tb-planning if I learn anything useful!

Comment 43

[WADA:World Anti-bad-Duping Agency]

(In reply to Andrew Sutherland [:asuth] from comment #42)
>    C: A152 NOOP
>    S: * VANISHED 8
>    S: A153 OK [HIGHESTMODSEQ 104] done

Oh, VANISHED response is replacement of EXPUNGE response.
I could know about pretty important feature of future QRESYNC support in Mozilla : correct VANISHED response processing.
Thanks again for your lecture.

Is solution I propsed in comment #40 correct and appropriate solution if without QRESYNC?
IIUC, sever's behavior is "process CHANGEDSINCE first" and "filter by uid xx:yy second", so I don't think "uid fetch 1:* flags (CHANGEDSINCe ...) every NN minutes" won't produce critical server workload issue", and "uid fetch 1:* flags without CHANGEDSINCE just after SELECT" is mandatory if QRESYNC is not used.

Comment 44

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

(In reply to WADA from comment #43)
> Is solution I propsed in comment #40 correct and appropriate solution if
> without QRESYNC?

Yes, although see below for a note about efficiency.

> IIUC, sever's behavior is "process CHANGEDSINCE first" and "filter by uid
> xx:yy second", so I don't think "uid fetch 1:* flags (CHANGEDSINCe ...)
> every NN minutes" won't produce critical server workload issue", and "uid
> fetch 1:* flags without CHANGEDSINCE just after SELECT" is mandatory if
> QRESYNC is not used.

Yes, your understanding of server behavior is consistent with my own understanding.  (Specifically, dovecot and Cyrus maintain transaction logs/journals)

If flag updates are being handled via CHANGEDSINCE, then it is potentially significantly faster and easier on the server to use a SEARCH command to get a list of the UIDs currently in the inbox.  And so there would never be any need to do a "uid fetch 1:* flags" without CHANGEDSINCE after initial sync of the folder.

For example:
  A8 UID SEARCH ALL
  * SEARCH 778 779 1419 2275 3522 4576 4577 4579 5270 5272 5274 5276 
  A8 OK UID SEARCH completed

Since servers will frequently already need to have a message sequence number mapping to UIDs efficiently available in memory, if optimized, this list of UIDs could be efficiently provided without having to look at any per-message data.

Comment 45

[WADA:World Anti-bad-Duping Agency]

(In reply to Andrew Sutherland [:asuth] from comment #44)
> For example:
>   A8 UID SEARCH ALL

Aha.
(i)   Flag state can be efficiently known by CONDSTORE any time.
(ii)  EXPUNGEd state while selecting Mbox can be efficiently known by QRESYNC.
(iii) Therefore, only "knowing about existing UID" is needed when imap client select the Mbox again at a connection.
       "UID not returned to UID SEARCH ALL" doesn't exist any more, so "simply remove it" is possible and sufficient.
It looks that client side implemetation will be simple if CONDSTORE+QRESYNC will be supported because all are UID basis.

Following is capability response from Gmail IMAP.
When will QRESYNC and UID EXPUNGE be supported by Gmail?
Why Gmail released CONDSTORE without QRESYNC? :-)
> * CAPABILITY IMAP4rev1 UNSELECT IDLE NAMESPACE QUOTA ID XLIST CHILDREN
>   X-GM-EXT-1 UIDPLUS COMPRESS=DEFLATE ENABLE MOVE CONDSTORE ESEARCH UTF8=ACCEPT 






> (In reply to WADA from comment #43)
> > Is solution I propsed in comment #40 correct and appropriate solution if
> > without QRESYNC?
> 
> Yes, although see below for a note about efficiency.
> 
> > IIUC, sever's behavior is "process CHANGEDSINCE first" and "filter by uid
> > xx:yy second", so I don't think "uid fetch 1:* flags (CHANGEDSINCe ...)
> > every NN minutes" won't produce critical server workload issue", and "uid
> > fetch 1:* flags without CHANGEDSINCE just after SELECT" is mandatory if
> > QRESYNC is not used.
> 
> Yes, your understanding of server behavior is consistent with my own
> understanding.  (Specifically, dovecot and Cyrus maintain transaction
> logs/journals)
> 
> If flag updates are being handled via CHANGEDSINCE, then it is potentially
> significantly faster and easier on the server to use a SEARCH command to get
> a list of the UIDs currently in the inbox.  And so there would never be any
> need to do a "uid fetch 1:* flags" without CHANGEDSINCE after initial sync
> of the folder.
> 
> For example:
>   A8 UID SEARCH ALL
>   * SEARCH 778 779 1419 2275 3522 4576 4577 4579 5270 5272 5274 5276 
>   A8 OK UID SEARCH completed
> 
> Since servers will frequently already need to have a message sequence number
> mapping to UIDs efficiently available in memory, if optimized, this list of
> UIDs could be efficiently provided without having to look at any per-message
> data.

Comment 46

[rsx11m]

Comment on attachment 8549951 [details] [diff] [review]
Just flip the default for now

Seeing that beta hasn't been tagged yet, nominating for the branches to give this some exposure in the betas prior to 38 ESR (in case someone is using an IMAP server for which condstore actually works properly and sees some other adverse effects now).

[Approval Request Comment]
Regression caused by (bug #): feature implemented and enabled in bug 436151
User impact if declined: continuing misbehavior with several IMAP servers
Testing completed (on c-c, etc.): c-c, but personally running with this setting for months now
Risk to taking this patch (and alternatives if risky): theoretically low

Comment 47

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/9aa8be8f7da3 -> FIXED

Comment 48

[Kent James (:rkent)]

https://hg.mozilla.org/releases/comm-aurora/rev/905018b6bc77
https://hg.mozilla.org/releases/comm-beta/rev/3c3d7829f5f3