Large Mailbox Append Fix

[Bron Gondwana]

On Mon, 24 Sep 2007 18:45:28 -0400, "Ken Murchison" <murch at andrew.cmu.edu> said:
> Bron Gondwana wrote:
> > On Mon, 24 Sep 2007 13:53:28 -0400, "Ken Murchison" <murch at andrew.cmu.edu> said:
> >> Bron Gondwana wrote:
> >>> On Sat, 22 Sep 2007 10:44:55 +0100 (BST), "David Carter" <dpc22 at cam.ac.uk> said:
> >>>> On Sat, 22 Sep 2007, Bron Gondwana wrote:
> >>>>
> >>>>> We still need to sync the final last_uid, which may be greater than
> >>>>> the UID of the maximum message.
> >>>> Oops, yes. Replacing last_uid for the last batch should be good enough.
> >>>>
> >>>>> (though I think there's a separate check later that does that if they
> >>>>> still don't match... I really should test-case this!)
> >>>> I don't think so. There is a separate test for UIDLAST if 
> >>>> do_mailbox_work() finds that it has no messages to UPLOAD.
> >>> Yeah, you're right.  I've got a simpler rewrite of the patch here that
> >>> does update_uidlast(mailbox) at the end if there's nothing in the
> >>> index_list.
> >>
> >> Attached is my rewrite against CVS.  I removed duplicate code that 
> >> resulted from having one for() loop with a special case if() inside (It 
> >> not uses a for() loop nested inside a do() loop).  The patch doesn't 
> >> look simpler because my version of diff appears to be braindead.
> > 
> > This still doesn't guarantee to update UIDLAST in the pathological case
> > where there are exactly max_count messages to be uploaded, not including
> > the final message in the list due to a UUID/GUID matchup - at least not
> > according to my reading of it.
> 
> Hmm.  I could be wrong, but UIDLAST always gets set in the last batch, 
> even if the number of messages is a multiple of max_count.  It may be 
> hard to tell from the diff, since its kind of chopped up.

Wow, and it's a pain to apply (don't suppose you could generate patches
with -p1 easily could you?  Quilt likes them that way).  Even applying by
hand though, it's breaking all over the place!  I'll try and hand import
it and see what it looks like.

I still read it as possible failing.

Can you please consider the following cases:

Replica last_uid = 2000

Master last_uid = 3005 (say, with the top couple of messages either deleted
or just happen to already exist on the other end for some reason like they
were copied in from another folder)

max_count = 1000

a) There are 999 messages which don't exist on the replica, the uid of the
   highest is 3002

b) There are 1000 messages which don't exist on the replica, the uid of the
   highest is 3003

c) There are 1001 messages which don't exist on the replica, the uid of the
   highest is 3004


In each case, what commands are sent?  I see the following:

a) UPLOAD 3005 <date> [999 messages]\r\n
b) UPLOAD 3003 <date> [1000 messages]\r\n
c) UPLOAD 3003 <date> [1000 messages]\r\n
   UPLOAD 3005 <date> [1 message]\r\n

It's the second case I'm worried about.  Also note that:

b) UPLOAD 3003 <date> [1000 messages]\r\n
   UPLOAD 3005 <date>\r\n

Will NOT work because the other end doesn't cope with an
empty UPLOAD command.  I also spun a patch which changed that, but
it looked more complex and harder to understand.

My code did (latest patch I sent):

b) UPLOAD 3003 <date> [1000 messages]\r\n
   UIDLAST 3005 <date>\r\n

Which wasn't perfectly efficient, but was correct.

Bron.
-- 
  Bron Gondwana
  brong at fastmail.fm