Mailbox item iterator pattern

Ken Murchison murch at andrew.cmu.edu
Tue Jun 19 12:03:57 EDT 2007


I'm away from my desk today so I can't check for sure, but I just recently fixed the RENAME bug, and I believe that the replication bug has been fixed too (patch from Umich).  Check bugzilla and/or CVS.

(from my phone)
-- 
Kenneth Murchison
Systems Programmer
Project Cyrus Developer/Maintainer
Carnegie Mellon University

-----Original Message-----
From: "Bron Gondwana" <brong at fastmail.fm>
To: cyrus-devel at lists.andrew.cmu.edu
Cc: "Rob Mueller" <robm at fastmail.fm>; "Ken Murchison" <murch at andrew.cmu.edu>; "David Carter" <dpc22 at cam.ac.uk>; "Wesley Craig" <wes at umich.edu>
Sent: 6/19/07 11:06 AM
Subject: Mailbox item iterator pattern

Hi,

I'm investigating restoring from backups as part of our
new backup system, and one problem is that recovering
seen state is basically impossible once a message has
been "expunged", even with delayed deletion.  This is
because index_checkseen walks the UID list in the
mailbox and joins across the gaps.

There are also other places with problems:

sync_client/sync_server - doesn't copy messages which
have been expunged.

mailbox_rename_copy - doesn't move messages or the
cyrus.expunge file.

Basically it looks like you need to open both files at
once and step through them - reading whichever is lowest
next.

I really, _really_ think that hacking that directly in to
every place that needs it is a bad idea for maintainability,
so I'd like to propose refactoring all the places that
iterate over a mailbox.


I've CC'ed a few people who I know are active patchers/
maintainers of Cyrus and who I've spoken with before
about Cyrus changes - but I'd especially appreciate
input from Ken about how best to do this.  The current
pattern I'm leaning towards is:


struct index_iter *iter;
struct index_record *record;
int flags = INBOX_ITER_EXPUNGED;

iter = mailbox_iterator_create(mailbox, flags);

while (record = mailbox_iterator_next(&iter)) {
  /* process things the same way you would have with the pattern:
     for (msgno = 1; msgno <= mailbox->exists; msgno++) {
         r = mailbox_read_index_record(mailbox, msgno, &record);
   */
}

r = mailbox_iterator_haserror(&iter);

mailbox_iterator_free(&iter);

return(r);

Obviously, if there was an error then mailbox_iterator_next will
return NULL, breaking the loop early.  You'll know what the
error is because 'r' gets set.


An alternative is:

struct index_iter iter;
struct index_record record;

mailbox_iterator_init(&iter, mailbox);
// or mailbox_iterator_initexpunge(&iter, mailbox);

while (mailbox_iterator_has_more_records(&iter)) {
  r = mailbox_iterator_next_record(&iter, &record);
  if (r) {
    mailbox_iterator_free(&iter);
    return r;
  }
  /* process */
}
mailbox_iterator_free(&iter);


I've deliberately left mailbox_iter unformatted, but it would
probably contain at least two index_record buffers, a locked
mmaped fd for each of mailbox and expunge files and some sort
of state tracking mechanism to say which one is next!

What do you think?  Is there value in either of the above?
Can you see a better way?

Bron.




More information about the Cyrus-devel mailing list