FastMail Cyrus Patches for upstream
brong at fastmail.fm
Thu Aug 27 22:30:22 EDT 2009
On Thu, 27 Aug 2009 21:19 -0400, "Brian Awood" <bawood at umich.edu> wrote:
> 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
Yep - that's pretty much how the CRC32 on the cache records
> > 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
Yeah - it does make a bit more sense when you put it like that.
Does it make 8 bytes per record worth of sense though? (we need
to pad to 8 byte multiples for 64 bit modseq alignment.) I'll have
another look at that in a second.
> > 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.
Right - that would be a bug then. I'll poke around the code.
Just to confirm - the symptom is copied messages out of
order causing the intermediate cache records to be 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.
Yeah, that's the problem though, because if the copied-back records
might have the wrong offsets unless everything was kept in sync
> > 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.
Cool. Sounds good! We've been OK with cache files for a while now,
but we had a RAID incident (tech pulled the wrong drive) which made
us want better integrity checking just-in-case[tm].
brong at fastmail.fm
More information about the Cyrus-devel