Simon Matter wrote:

I've tried to get rid of hardlinked files in our cyrus spools to make
partial mailbox restores easier and more safe. My first idea was to set
"singleinstancestore: no" in imapd.conf and make all payload files
independant from each other. Unfortunately I realized later that while
incoming messages are stored in independant files, the IMAP COPY command
still creates hardlinked message files. Of course there is nothing wrong
with singleinstancestore because the manpage clearly states that only
delivery via lmtp/nntp is affected.
I have now looked at the code and found that mailbox_copyfile() has a
nolink parameter which controls the copy/link behaviour. My idea was to
create a new config option to disable hardlinks completely.

My questions:
- is such an option a very bad idea?
- does such a patch already exist somewhere?
- what's the best name of a new config option?
- is there a chance to have such a patch accepted into the distribution?


I'd like to include the following patch into my cyrus-imapd rpm packages
to address the issue mentioned above. While testing it on a test box it
seemed to work very well. Do the cyrus developers see any possible problem
with it?

Actually, your patch affects ALL mailbox_copyfile(), which means that messages won't even be hardlinked from the stage./ to the destination mailbox. I don't think we want to do this, since this will hurt performance for all messages deliveries (even single recipient messages).

IMO, singleinstancestore *should* also govern IMAP COPY (does anyone object to this assessment?), so I'd propose the following patch:

Index: imap/append.c
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/append.c,v
retrieving revision 1.107
diff -u -r1.107 append.c
--- imap/append.c       22 May 2004 03:45:48 -0000      1.107
+++ imap/append.c       2 Jun 2005 14:27:39 -0000
@@ -801,7 +801,8 @@
 int append_copy(struct mailbox *mailbox,
                struct appendstate *as,
                int nummsg,
-               struct copymsg *copymsg)
+               struct copymsg *copymsg,
+               int nolink)
 {
     struct mailbox *append_mailbox = &as->m;
     int msg;
@@ -845,7 +846,7 @@
            mailbox_message_get_fname(mailbox, copymsg[msg].uid, fnamebuf,
                                      sizeof(fnamebuf));
            /* Link/copy message file */
-           r = mailbox_copyfile(fnamebuf, fname, 0);
+           r = mailbox_copyfile(fnamebuf, fname, nolink);
            if (r) goto fail;

            /* Write out cache info, copy other info */
Index: imap/append.h
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/append.h,v
retrieving revision 1.26
diff -u -r1.26 append.h
--- imap/append.h       22 Jan 2004 21:17:07 -0000      1.26
+++ imap/append.h       2 Jun 2005 14:27:39 -0000
@@ -137,7 +137,7 @@

 extern int append_copy(struct mailbox *mailbox,
                       struct appendstate *append_mailbox,
-                      int nummsg, struct copymsg *copymsg);
+                      int nummsg, struct copymsg *copymsg, int nolink);

 extern int append_collectnews(struct appendstate *mailbox,
                              const char *group, unsigned long feeduid);
Index: imap/imapd.c
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/imapd.c,v
retrieving revision 1.492
diff -u -r1.492 imapd.c
--- imap/imapd.c        27 May 2005 14:57:41 -0000      1.492
+++ imap/imapd.c        2 Jun 2005 14:27:39 -0000
@@ -3605,7 +3605,7 @@
                                               imapd_userid, mailboxname);
     if (!r) {
        r = index_copy(imapd_mailbox, sequence, usinguid, mailboxname,
-                      &copyuid);
+ &copyuid, !config_getswitch(IMAPOPT_SINGLEINSTANCESTORE))
;
     }

     index_check(imapd_mailbox, usinguid, 0);
Index: imap/imapd.h
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/imapd.h,v
retrieving revision 1.61
diff -u -r1.61 imapd.h
--- imap/imapd.h        22 Jun 2004 21:36:18 -0000      1.61
+++ imap/imapd.h        2 Jun 2005 14:27:39 -0000
@@ -249,7 +249,7 @@
 extern int index_thread(struct mailbox *mailbox, int algorithm,
                        struct searchargs *searchargs, int usinguid);
 extern int index_copy(struct mailbox *mailbox, char *sequence,
-                        int usinguid, char *name, char **copyuidp);
+ int usinguid, char *name, char **copyuidp, int nolink);
 extern int index_status(struct mailbox *mailbox, char *name,
                           int statusitems);

Index: imap/index.c
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/index.c,v
retrieving revision 1.217
diff -u -r1.217 index.c
--- imap/index.c        27 May 2005 14:57:55 -0000      1.217
+++ imap/index.c        2 Jun 2005 14:27:39 -0000
@@ -1155,7 +1155,8 @@
           char *sequence,
           int usinguid,
           char *name,
-          char **copyuidp)
+          char **copyuidp,
+          int nolink)
 {
     static struct copyargs copyargs;
     int i;
@@ -1188,7 +1189,7 @@
     docopyuid = (append_mailbox.m.myrights & ACL_READ);

     r = append_copy(mailbox, &append_mailbox, copyargs.nummsg,
-                   copyargs.copymsg);
+                   copyargs.copymsg, nolink);
     if (!r) append_commit(&append_mailbox, totalsize,
                          &uidvalidity, &startuid, &num);
     if (!r && docopyuid) {
Index: lib/imapoptions
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/lib/imapoptions,v
retrieving revision 1.32
diff -u -r1.32 imapoptions
--- lib/imapoptions     11 Apr 2005 06:57:31 -0000      1.32
+++ lib/imapoptions     2 Jun 2005 14:27:40 -0000
@@ -776,9 +776,9 @@
    directories: ~user/.sieve. */

 { "singleinstancestore", 1, SWITCH }
-/* If enabled, lmtpd and nntpd attempt to only write one copy of a message per
-   partition and create hard links, resulting in a potentially large
-   disk savings. */
+/* If enabled, imapd, lmtpd and nntpd attempt to only write one copy
+   of a message per partition and create hard links, resulting in a
+   potentially large disk savings. */

 { "skiplist_unsafe", 0, SWITCH }
 /* If enabled, this option forces the skiplist cyrusdb backend to


--
Kenneth Murchison     Oceana Matrix Ltd.
Software Engineer     21 Princeton Place
716-662-8973 x26      Orchard Park, NY 14127
--PGP Public Key--    http://www.oceana.com/~ken/ksm.pgp
---
Cyrus Home Page: http://asg.web.cmu.edu/cyrus
Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html

Reply via email to