On Fri, 5 Dec 2008 08:25:38 +0100 (CET), "Simon Matter" <simon.matter@xxxxxxxxx> wrote:
> > Hi all,
> >
> > Cyrus stores the end of the cache file before starting an
> > append operation so that it can truncate back to that point
> > if the append is aborted.
> >
> > Unfortunately, it actually stores cache_len rather than
> > cache_size. That sort of sucks, because cache_len is
> > rounded up by quite a bit to allow "slop". As the cache
> > file gets bigger, the slop gets bigger too, and you wind
> > up with a whole pile of zero blocks in your cache file,
> > making it (even if sparse) somewhat massive.
> >
> > Oh, and the bogus record(s) that you wrote are going to
> > still be (possibly only partially) in the file, because
> > the truncate will either be past them, or in the middle
> > of them.
> >
> > This is exacerbated by the fact that duplicate suppression
> > seems to need to write to the cache file _before_ it decides
> > not to accept the message!
> >
> > The attached patch fixes the issue, adds a comment, and
>
> Hi Bron,
>
> is there something missing in the patch, because I can't see the "adds a
> comment" part?
Hmm... could be. Damn. I'll just go back and have a look. I did the
comment after the rest of it - it's pretty meaningless anyway. The code
in the patch is certainly fine...
Ok - here's the copy with the comment!
Bron.
--
Bron Gondwana
brong@xxxxxxxxxxx
Index: cyrus-imapd-2.3.13/imap/append.c
===================================================================
--- cyrus-imapd-2.3.13.orig/imap/append.c 2008-12-05 02:56:42.000000000 +0000
+++ cyrus-imapd-2.3.13/imap/append.c 2008-12-05 02:58:44.000000000 +0000
@@ -234,8 +234,10 @@ int append_setup(struct appendstate *as,
as->userid[0] = '\0';
}
+ /* store original size to truncate if append is aborted */
+ as->orig_cache_size = as->m.cache_size;
+
/* zero out metadata */
- as->orig_cache_len = as->m.cache_len;
as->nummsg = as->numanswered =
as->numdeleted = as->numflagged = 0;
as->quota_used = 0;
@@ -369,7 +371,7 @@ int append_abort(struct appendstate *as)
}
/* truncate the cache */
- ftruncate(as->m.cache_fd, as->orig_cache_len);
+ ftruncate(as->m.cache_fd, as->orig_cache_size);
/* unlock mailbox */
mailbox_unlock_index(&as->m);
Index: cyrus-imapd-2.3.13/imap/append.h
===================================================================
--- cyrus-imapd-2.3.13.orig/imap/append.h 2008-12-05 02:56:42.000000000 +0000
+++ cyrus-imapd-2.3.13/imap/append.h 2008-12-05 02:57:12.000000000 +0000
@@ -76,7 +76,7 @@ struct appendstate {
/* current state of append */
/* if we abort, where should we truncate the cache file? */
- unsigned long orig_cache_len;
+ unsigned long orig_cache_size;
int writeheader; /* did we change the mailbox header? */
----
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
