From: Simon Matter (no email)
Date: Thu Jun 02 2005 - 10:54:10 EDT
> 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).
That's why I was asking whether it's a good idea. IIRC mailbox_copyfile()
is also used to handle other files, not only message files. So my solution
was a dirty hack.
>
> IMO, singleinstancestore *should* also govern IMAP COPY (does anyone
That's what I expected first but found that it's only implemented for
incoming messages for unknown reason. Your proposed solution looks like
the way to go.
Simon
> 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,
> - ©uid);
> + ©uid,
> !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
|
|
|