Patches used at FastMail.FM

Bron Gondwana brong at fastmail.fm
Tue Jan 9 16:41:46 EST 2007


On Tue, 09 Jan 2007 12:49:25 -0500, "Ken Murchison" <murch at andrew.cmu.edu> said:
> Bron Gondwana wrote:
> > It's been a while since I posted our list of patches, and there have been
> > a couple of changes since then.
> > 
> > I'm generating the site from a script and a bunch of patch description files
> > now, so I should be able to keep it up to date.
> > 
> > 
> > Feel free to use any of these patches, and Ken - feel free to include
> > anything that looks good into upstream!  Some of these patches are quite
> > specific to our site, but many of the are more generally useful as well.
> 
> After a quick discussion with Jeff, I just committed the following 
> patches to CVS.  Others may follow for a later 2.3 release or rolled 
> into 2.4.
>
> [...]

Wow, makes me wish I'd got sorted and split out the patch that you accepted
about half of already so I could ask why the other half didn't go in.  I've
attached the entire patch now.  I'm leaving on vacation in a couple of hours
so my wife would kill me if I spent any longer on this now...

This is the one that stops cyr_expire performing so many index opens and
closes by checking if the cyrus_expunge file exists first.  You appear to
have accepted the code that does that, but not the code for the additional
option (-a) which skips reading the annotations database.  I haven't
actually done a performance comparison, so I can't guarantee that it is
a big performance win, but the strace I did showed about 4 db lookups per
folder (because it walked back up the tree).  We don't use annotations at
all, at least not for the purpose of controlling expiry, so it's somewhat
of a performance saving.

Anyway, thanks heaps for looking at those patches and taking so many of them.
It's always good to reduce our "distance" from upstream.  We are still
going to be patching cyrus a little bit (you'll notice that I didn't include
cyrus-fastmailsecrects.diff on the website!) - at least until we find some
other way to add our s00per-s33krit encrypted header to messages which
allows us to know which user (or which user's sieve script) generated a
message without leaking their identity to any third party.

Regards,

Bron.
-- 
  Bron Gondwana
  brong at fastmail.fm

-------------- next part --------------
diff -ur --new-file cyrus-imapd-cvs.orig/imap/cyr_expire.c cyrus-imapd-cvs/imap/cyr_expire.c
--- cyrus-imapd-cvs.orig/imap/cyr_expire.c	2005-12-15 08:21:16.000000000 -0500
+++ cyrus-imapd-cvs/imap/cyr_expire.c	2006-10-11 04:26:12.000000000 -0400
@@ -73,7 +73,7 @@
 void usage(void)
 {
     fprintf(stderr,
-	    "cyr_expire [-C <altconfig>] -E <days> [-X <expunge-days>] [-v]\n");
+	    "cyr_expire [-C <altconfig>] -E <days> [-X <expunge-days>] [-a] [-v]\n");
     exit(-1);
 }
 
@@ -86,6 +86,7 @@
     unsigned long messages;
     unsigned long deleted;
     int verbose;
+    int skip_annotate;
 };
 
 /*
@@ -143,26 +144,32 @@
      * since mailboxes inherit /vendor/cmu/cyrus-imapd/expire,
      * we need to iterate all the way up to "" (server entry)
      */
-    while (1) {
-	r = annotatemore_lookup(buf, "/vendor/cmu/cyrus-imapd/expire", "",
-				&attrib);
-
-	if (r ||				/* error */
-	    attrib.value ||			/* found an entry */
-	    !buf[0] ||				/* done recursing */
-	    !strcmp(buf+domainlen, "user")) {	/* server entry doesn't apply
-						   to personal mailboxes */
-	    break;
-	}
-
-	p = strrchr(buf, '.');			/* find parent mailbox */
-
-	if (p && (p - buf > domainlen))		/* don't split subdomain */
-	    *p = '\0';
-	else if (!buf[domainlen])		/* server entry */
-	    buf[0] = '\0';
-	else					/* domain entry */
-	    buf[domainlen] = '\0';
+    if (erock->skip_annotate) {
+      /* we don't want to check for annotations, so we didn't find any */
+      attrib.value = 0;
+    }
+    else {
+        while (1) {
+	    r = annotatemore_lookup(buf, "/vendor/cmu/cyrus-imapd/expire", "",
+				    &attrib);
+    
+	    if (r ||				/* error */
+	        attrib.value ||			/* found an entry */
+	        !buf[0] ||				/* done recursing */
+	        !strcmp(buf+domainlen, "user")) {	/* server entry doesn't apply
+						       to personal mailboxes */
+	        break;
+	    }
+    
+	    p = strrchr(buf, '.');			/* find parent mailbox */
+    
+	    if (p && (p - buf > domainlen))		/* don't split subdomain */
+	        *p = '\0';
+	    else if (!buf[domainlen])		/* server entry */
+	        buf[0] = '\0';
+	    else					/* domain entry */
+	        buf[domainlen] = '\0';
+        }
     }
 
     if (!r && (attrib.value ||
@@ -170,9 +177,24 @@
 	struct mailbox mailbox;
 	int doclose = 0;
 	int expunge_flags = 0;
+	char *path, *mpath, *acl, fnamebuf[MAX_MAILBOX_PATH+1];
+	struct stat statbuf;
+
+	/* does the file even exist?  Saves IO hit of opening header */
+	r = mboxlist_detail(name, NULL, &path, &mpath, NULL, &acl, NULL);
+	if (r) return r;
+	if (mpath &&
+	   (config_metapartition_files & IMAP_ENUM_METAPARTITION_FILES_EXPUNGE)) {
+	   strlcpy(fnamebuf, mpath, sizeof(fnamebuf));
+	}
+	else
+	    strlcpy(fnamebuf, path, sizeof(fnamebuf));
+	strlcat(fnamebuf, FNAME_EXPUNGE_INDEX, sizeof(fnamebuf));
+	r = stat(fnamebuf, &statbuf);
+	if (r) return 0;  /* file doesn't exist */
 
-	/* Open/lock header */
-	r = mailbox_open_header(name, 0, &mailbox);
+	/* Open/lock header - we already have paths skip lookup */
+	r = mailbox_open_header_path(name, path, mpath, acl, 0, &mailbox, 0);
 	if (!r && mailbox.header_fd != -1) {
 	    doclose = 1;
 	    (void) mailbox_lock_header(&mailbox);
@@ -242,7 +264,7 @@
     /* zero the expire_rock */
     memset(&erock, 0, sizeof(erock));
 
-    while ((opt = getopt(argc, argv, "C:E:X:v")) != EOF) {
+    while ((opt = getopt(argc, argv, "C:E:X:va")) != EOF) {
 	switch (opt) {
 	case 'C': /* alt config file */
 	    alt_config = optarg;
@@ -261,6 +283,10 @@
 	case 'v':
 	    erock.verbose++;
 	    break;
+
+	case 'a':
+	    erock.skip_annotate = 1;
+	    break;
 	
 	default:
 	    usage();
diff -ur --new-file cyrus-imapd-cvs.orig/man/cyr_expire.8 cyrus-imapd-cvs/man/cyr_expire.8
--- cyrus-imapd-cvs.orig/man/cyr_expire.8	2004-04-03 13:44:55.000000000 -0500
+++ cyrus-imapd-cvs/man/cyr_expire.8	2006-10-11 04:27:57.000000000 -0400
@@ -96,6 +96,11 @@
 .TP
 .B \-v
 Enable verbose output.
+.TP
+.B \-a
+Skip the annotation lookup, so all \fB/vendor/cmu/cyrus-imapd/expire\fR
+annotations are ignored entirely.  It behaves as if they were not set, so
+only \fIexpire-days\fR is considered for all mailboxes.
 .SH FILES
 .TP
 .B /etc/imapd.conf


More information about the Info-cyrus mailing list