FastMail Cyrus Patches for upstream

[Brian Awood]

On Thursday 27 August 2009 @ 18:55, Bron Gondwana wrote:
>
> a) it's a pain to update.  For cache_crc I've special-cased "0" so
> we don't have to read every single cache record.  You'd have to do
> the same here, or have an "options" value like pop3_uidl so that a
> mailbox could be "upgraded" during a process_records run or
> something.  Don't worry, I've already considered this for my "get
> shared seen back into the index file rather than in a bloody huge
> non-findable skiplist file that can't be replicated to a new server
> easily".  It's doable.

True, it would be a pain to update existing files.  A low impact way 
might be to only generate it during things like an append or 
reconstruct.

> b) it's un-necessary.  If the record is not broken, then parsing
> out the length only takes 10 operations, and it's not like we have
> to do it that often.

Also true,  but if you had the length, it could be: read cache data of 
length x, calculate the crc of the buffer, verify and then parse it 
if it's valid.  I suppose it's 6 of one and a 1/2 dozen of the other, 
it just makes more sense to me if the data was verified before 
parsing.

> Ahh - there were a couple of bugs.  One that wrote everything
> between two records to the file, and another that truncated
> incorrectly on failed append, leaving all sorts of extra junk in
> cache files.  They should both be fixed in 2.3.14 I think.  Though
> cache files could still contain junk on disk until a rewrite gets
> the whole thing!

We're running 2.3.14 + local & some Fastmail patches.  I think the 
truncation of the file on a failed append works correctly, but it 
seems like extra records are still getting copied.  

> Unexpunge is the only thing I can think of that might give out of
> order cache records.  Though I _think_ it may actually be totally
> broken with respect to cache records at the moment.  I would
> recommend a reconstruct after any unexpunge at the moment.

unexpunge seems to be ok, based on some simple testing.  It doesn't 
touch the cache file at all, and seems to just be moving records back 
to the index.  I think we'll be reconstructing unexpunged mailboxes 
for now anyway.

> Ok - so that's a bug that needs fixing, if it's still present in
> CVS.
>
> I _thought_ I had fixed it.  Fixing this bug doesn't require having
> the record length in the index file though.  It can be done just by
> reading the record itself - which we now have a nice easy API for,
> which returns the length and does CRC and internal structure checks
> at the same time!

Great! I'm looking forward to testing out this code, cache files have 
been our main ongoing issue. 

Brian