137994 - IMAP downloadaing all headers instead of just headers required for d

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[David :Bienvenu]

Comment 1

[David :Bienvenu]

The fix for supporting custom headers accidentally broke the imap code which
determines which headers to download when opening a folder. We are now
downloading all headers instead of just the headers needed for the db when
opening a folder. I think this also hurts web-mail imap server performance since
we're going to ask for all headers. (If you turn on custom headers, everything
works like it used to, but very few people will turn on custom headers). I'll
attach a patch soon.

Comment 2

[David :Bienvenu]

Attachment: proposed fix

OK, there was some confusion in the code about what arbitrary headers mean, as
opposed to custom headers. I've cleaned that all up and changed the method name
so people won't get confused.

Comment 3

[David :Bienvenu]

Navin, can you review? thx.

Comment 4

[(not reading, please use [e-mail] instead)]

Comment on attachment 79673 [details] [diff] [review]
proposed fix

sr=sspitzer

Comment 5

[Navin Gupta]

Comment on attachment 79673 [details] [diff] [review]
proposed fix

r=naving

Comment 6

[Navin Gupta]

Comment on attachment 79673 [details] [diff] [review]
proposed fix

We know that we have not
implemented filters on
any headers. So why check
it, just take it to be	false


+	 PRBool downloadAllHeaders = PR_FALSE;
+	 GetShouldDownloadAllHeaders(&downloadAllHeaders); // checks if we're
filtering on "any header"
+	 if (!downloadAllHeaders)  // if it's ok -- no filters on any header,
etc.
	 {
	   char *headersToDL = nsnull;
	   char *what = nsnull;

Comment 7

[David :Bienvenu]

the filters code knows that, but the imap code doesn't know that - I can check
it in that way with a big comment, I guess, but it basically saves a function
call. But that doesn't affect the computation of the custom headers to download...

Comment 8

[Navin Gupta]

yes, I want to avoid that function call. you can comment that function call and 
say not implemented or something.

Comment 9

[scottputterman]

adding adt1.0.0 [adt2] for the performance hit for a very common mail operation.

Comment 10

[David :Bienvenu]

Karen, can you verify this fix today on the trunk, for normal imap servers
(e.g., the netscape server) and for webmail servers? We would like to check this
in soon since it's a rather serious regression from a performance point of view.
I'll attach two protocol logs, one before the fix, that shows us downloading all
the headers, and one after the fix, showing us just downloading the headers we
need for our db, and if you can just verify that your logs show the same thing,
that would be great.

Comment 11

[David :Bienvenu]

Attachment: this shows the old, bad behaviour

Comment 12

[David :Bienvenu]

Attachment: this shows the correct behaviour after the fix

this shows us just downloading the headers we need instead of all the headers.

Comment 13

[Jaime Rodriguez, Jr.]

Huang - Pls verify that we get a performance win on the trunk, and that this
does not cause any regressions.

Comment 14

[David :Bienvenu]

fixed on trunk

Comment 15

[Jaime Rodriguez, Jr.]

adt1.0.0-/adt2 RTM. This is good bug to take for RTM. Let's pick it up, after
MachV beta ships.

Comment 16

[David :Bienvenu]

karen, any luck verifying this? this has a very large performance impact on imap
servers, and on users with slow connections.

Comment 17

[grylchan]

Helping Karen & David out, trying to qa this on trunk.

I ran into problem with customized headers as I get a error
mesg about FETCH BODY.PEEK[HEADER.FIELDS every time i do a GetMsgs. 
See bug 155496.

Adding dependency.

Comment 18

[grylchan]

Using commercial trunk
2002-07-01-08-trunk/ NT 4.0
2002-07-02-08-trunk/ mac 9.2.2
2002-07-02-04-trunk/ linux 2.2

Looking at the protocol logs for am imap mail account.

I have confirmed the headers that we were downloading
are indeed smaller now when I go do a 'getmsg' to receive
a new mesg.

For comparisons sake:

Old way we downloaded all headers:
-INBOX:SendData: 15 UID fetch 257:258 (UID RFC822.SIZE BODY.PEEK[HEADER] FLAGS)
-INBOX:CreateNewLineFromSocket: * 64 FETCH (FLAGS (\Recent) UID 257 RFC822.SIZE 
708 BODY[HEADER] {700}
-INBOX:STREAM:OPEN Size: 708: Begin Message Download Stream
-INBOX:CreateNewLineFromSocket: Return-Path: <[e-mail]>
-INBOX:CreateNewLineFromSocket: Received: from linzilla.mcom.com 
([10.169.111.205]) by
-INBOX:CreateNewLineFromSocket:           linzilla.mcom.com (Netscape Messaging 
Server 4.15) with ESMTP id
-INBOX:CreateNewLineFromSocket:           GYLLAZ00.C11 for 
<[e-mail]>; Mon, 1 Jul 2002
-INBOX:CreateNewLineFromSocket:           18:25:47 -0700 
-INBOX:CreateNewLineFromSocket: Message-ID: <[e-mail]>
-INBOX:CreateNewLineFromSocket: Date: Mon, 01 Jul 2002 18:25:48 -0700
-INBOX:CreateNewLineFromSocket: From: "g chan6" <[e-mail]>
-INBOX:CreateNewLineFromSocket: User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; 
en-US; rv:1.0.1) Gecko/20020701 Netscape/7.0
-INBOX:CreateNewLineFromSocket: X-Accept-Language: en-us, en
-INBOX:CreateNewLineFromSocket: MIME-Version: 1.0
-INBOX:CreateNewLineFromSocket: To: g chan6 <[e-mail]>
-INBOX:CreateNewLineFromSocket: Subject: superboy
-INBOX:CreateNewLineFromSocket: Content-Type: text/plain; charset=us-ascii; 
format=flowed
-INBOX:CreateNewLineFromSocket: Content-Transfer-Encoding: 7bit
-INBOX:CreateNewLineFromSocket: 
-INBOX:CreateNewLineFromSocket: )
-INBOX:STREAM:CLOSE: Normal Message End Download Stream

With the fix, the new way we download headers (notice less lines):


-INBOX:SendData: 14 UID fetch 264 (UID RFC822.SIZE FLAGS BODY.PEEK[HEADER.FIELDS 
(From To Cc Subject Date Message-ID Priority X-Priority References Newsgroups)])
-INBOX:CreateNewLineFromSocket: * 71 FETCH (FLAGS (\Recent) UID 264 RFC822.SIZE 
709 BODY[HEADER.FIELDS (From To Cc Subject Date Message-ID Priority X-Priority 
References Newsgroups)] {194}
-INBOX:STREAM:OPEN Size: 709: Begin Message Download Stream
-INBOX:CreateNewLineFromSocket: Message-ID: <[e-mail]>
-INBOX:CreateNewLineFromSocket: Date: Tue, 02 Jul 2002 11:42:34 -0700
-INBOX:CreateNewLineFromSocket: From: "g chan6" <[e-mail]>
-INBOX:CreateNewLineFromSocket: To: "g chan6" <[e-mail]>
-INBOX:CreateNewLineFromSocket: Subject: shaggy
-INBOX:CreateNewLineFromSocket: 
-INBOX:CreateNewLineFromSocket: )
-INBOX:STREAM:CLOSE: Normal Message End Download Stream

Still not sure about custom headers and how it deals with webmail
(as I didn't see the 'peek' lines in protocol log as it relates
to webmail accounts)

Will leave it as Resolved for now and will do more testing
as instructed.

Comment 19

[David :Bienvenu]

for web mail, we use a different command to get the envelope information.

Comment 20

[grylchan]

so would log for webmail be different between current trunk & branch?
Anything I need to look for?

Comment 21

[Karen Huang]

I am investigating this bug right now.....

Comment 22

[Karen Huang]

Removing dependency bug 155496 since that bug should be different issue.

Verified on all the platforms for Netscape Messaging Server. Results are same as 
David & Gray described on above comments.
Veriifed on Windows 07-09-08-trunk build Messaging Server IMAP
Verified on Linux 07-09-07-trunk build Messaging Server IMAP
Verified on Mac 07-09-08-trunk build Messaging Server IMAP

Verified on all the platforms for Webmail, got the expected results as following 
Webmail IMAP log.
Veriifed on Windows 07-09-08-trunk build Webmail
Verified on Linux 07-09-07-trunk build Webmail
Verified on Mac 07-09-08-trunk build Webmail

Comment 23

[Karen Huang]

Attachment: Webmail old behaviour

Comment 24

[Karen Huang]

Marking as verified based on the confirmation with developer.

Comment 25

[Karen Huang]

Ccing Scott.
Should this bug change keywords from "adt 1.0.0-" to "adt1.0.1"?