[POLL] Development guidance

[Bron Gondwana]

On Fri, Nov 02, 2007 at 12:42:37PM -0400, Ken Murchison wrote:

Bah - sorry I didn't reply to this days ago.  I was fiddling with doing
stupid things to my cyrus.index files and managed to break the
UIDVALIDITY on my Lists.Cyrus folder, at which offlineimap promptly
stopped syncing the folder with only a small message amongst a large
pile of debugging output (yeah, I should run it quieter).  I didn't
realise I wasn't seeing Cyrus list traffic for a bit, though I did
think you guys were awfully quiet!

> I'm getting ready to implement the QRESYNC extension for the upcoming 
> LEMONADE interop.
>
> http://www.ietf.org/internet-drafts/draft-ietf-lemonade-reconnect-client-06.txt
>
> The major hurdle in implementing this extension is that we need to store 
> state for expunged messages.  Currently I see two ways to do this:
>
> 1. Leverage delayed expunge which already stores state for expunged 
> messages in cyrus.expunge (up until the records are purged by cyr_expire).
>
> 2. Create a separate database, or cyrus.index/cyrus.cache entry for storing 
> expunged UID-sets and MODSEQs.
>
> My gut tells me to just leverage cyrus.expunge.  The only downside that I 
> can see is that QRESYNC now requires the admin to enable delayed expunge, 
> and the length of the availability of the expunge data depends on how 
> frequently the admin decides to purge with cyr_expire.  However, as long as 
> we document this dependency, and give some recommended guidelines for 
> QRESYNC, this might be a good tradeoff.
>
> Thoughts?  Any other implementation ideas?

One cyrus.index file containing both expunged and non-expunged messages,
with a flag showing which they are.  It does mean that every time you
load a cyrus.index file, you need to build a structure in memory
telling you the offsets of the non-expunged messages if you want to
refer to them by position rather than UID, but most clients don't do
that already, and most imapd code just iterates over the entire file
anyway, so you'd just have something like:

for (msgno = 1; msgno <= nrecords; msgno++) {
  mailbox_read_index_record(mailbox, msgno, &record);
  if (record.system_flags & FLAG_EXPUNGED) continue;

  /* process record */
}

/* REPLACES */
#if 0
for (msgno = 1; msgno <= exists; msgno++) {
  /* not using MACRO(msgno) any more because it won't
     work for crc32, and besides it used global variables */
  mailbox_read_index_record(mailbox, msgno, &record);

  /* process record */ 
}
#endif

(I'm assuming you wouldn't want to add another 8 bytes just to
 deal with expungedness)

This would allow vast gobs of complexity with handling cyrus.expunge
files to be removed.  It would give free "deleted messages get synced"
support to the replication protocol.

It would mean that NOTHING gets O(N) rewritten when you delete a message
from a mailbox (at the moment the index record just gets appended to
the cyrus.expunge, but the cyrus.index gets rewritten entirely), only
when cyr_expire is run.

There would be a new counter (nexpunged) added to the header of the
cyrus.index file, so you could still get a real exists count by some
form of maths on them (either nrecords == exists + nexpunged) or
realexists = nrecords - nexpunged - with nrecords being a new name 
for what is currently "exists".

cyr_expire would of course rewrite the entire index file when it
decided it was needed.  First it would remove the underlying files,
then do the rewrite, so that if it failed part way through a
reconstruct would notice and just silently drop those records.

Oh - and since things would remain sorted in UID order there
wouldn't be all the ugly sorting issue that cyrus.expunge 
creates every time you have to deal with it.


I think it would be a huge win for maintainability and simplicity
of all the code that has to deal with index records and be aware
of the delayed expunge.  On to how it helps you with the QRESYNC:

(a) make qresync require some form of delayed_expunge.
(b) create a new delayed_expunge mode (keep index record, nuke file).
(c) make reconstruct keep the index record as a deleted record if
    the underlying file is missing.
(d) make cyr_expire default to keeping the index record for whatever
    time qresync needs if qresync is turned on (but still delete the
    underlying files per the command line flag).

I'm already touching large parts of the necessary code with my
crc32 index record integrity project, and Rob is getting sick of
me harping on about how much I want this method to be chosen.  
Hopefully you'll get sick of it too and choose my way :)

Bron.