Cache truncation bug on aborted appends

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
renames the temporary variable to reflect the value it's
actually storing.

Wes/Ken, please apply to CVS for the next stable release.

Everyone else, I recommend you apply this patch.  We have
had to reconstruct the occasional mailbox as their cache file
size spirals out of control and the process hits memory
limits trying to map it.

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:56:58.000000000 +0000
@@ -235,7 +235,7 @@ int append_setup(struct appendstate *as,
     }
 
     /* zero out metadata */
-    as->orig_cache_len = as->m.cache_len;
+    as->orig_cache_size = as->m.cache_size;
     as->nummsg = as->numanswered = 
 	as->numdeleted = as->numflagged = 0;
     as->quota_used = 0;
@@ -369,7 +369,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

[Index of Archives]     [Cyrus SASL]     [Squirrel Mail]     [Asterisk PBX]     [Video For Linux]     [Photo]     [Yosemite News]     [gtk]     [KDE]     [Gimp on Windows]     [Steve's Art]

  Powered by Linux