4b/ef1485b1606a852c17caa8ff90bf5937306ab2

   1 Return-Path: <m.walters@qmul.ac.uk>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 99B81431FAF\r
   6         for <notmuch@notmuchmail.org>; Sun, 25 Nov 2012 08:15:55 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id vSwYyDBotsF6 for <notmuch@notmuchmail.org>;\r
  17         Sun, 25 Nov 2012 08:15:53 -0800 (PST)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id BCD32431FAE\r
  22         for <notmuch@notmuchmail.org>; Sun, 25 Nov 2012 08:15:52 -0800 (PST)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1Tcerz-00065i-T6; Sun, 25 Nov 2012 16:15:48 +0000\r
  27 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost)\r
  28         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
  29         (envelope-from <m.walters@qmul.ac.uk>)\r
  30         id 1Tcerz-0001Wa-H7; Sun, 25 Nov 2012 16:15:47 +0000\r
  31 From: Mark Walters <markwalters1009@gmail.com>\r
  32 To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org\r
  33 Subject: Re: [PATCH v2 15/20] insert: fsync new directory after rename\r
  34 In-Reply-To: <1353806206-29133-16-git-send-email-novalazy@gmail.com>\r
  35 References: <1353806206-29133-1-git-send-email-novalazy@gmail.com>\r
  36         <1353806206-29133-16-git-send-email-novalazy@gmail.com>\r
  37 User-Agent: Notmuch/0.14+81~g9730584 (http://notmuchmail.org) Emacs/23.4.1\r
  38         (x86_64-pc-linux-gnu)\r
  39 Date: Sun, 25 Nov 2012 16:15:50 +0000\r
  40 Message-ID: <87624tk6a1.fsf@qmul.ac.uk>\r
  41 MIME-Version: 1.0\r
  42 Content-Type: text/plain; charset=us-ascii\r
  43 X-Sender-Host-Address: 93.97.24.31\r
  44 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  45 X-QM-Body-MD5: dd4d33770611628382466e9c89ac22ec (of first 20000 bytes)\r
  46 X-SpamAssassin-Score: -1.8\r
  47 X-SpamAssassin-SpamBar: -\r
  48 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  49         determine if it is\r
  50         spam. We require at least 5.0 points to mark a message as spam.\r
  51         This message scored -1.8 points.\r
  52         Summary of the scoring: \r
  53         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
  54         *      medium trust\r
  55         *      [138.37.6.40 listed in list.dnswl.org]\r
  56         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  57         provider *      (markwalters1009[at]gmail.com)\r
  58         *  0.5 AWL AWL: From: address is in the auto white-list\r
  59 X-QM-Scan-Virus: ClamAV says the message is clean\r
  60 X-BeenThere: notmuch@notmuchmail.org\r
  61 X-Mailman-Version: 2.1.13\r
  62 Precedence: list\r
  63 List-Id: "Use and development of the notmuch mail system."\r
  64         <notmuch.notmuchmail.org>\r
  65 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  66         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  67 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  68 List-Post: <mailto:notmuch@notmuchmail.org>\r
  69 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  70 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  72 X-List-Received-Date: Sun, 25 Nov 2012 16:15:55 -0000\r
  73 \r
  74 On Sun, 25 Nov 2012, Peter Wang <novalazy@gmail.com> wrote:\r
  75 > After moving the file from the 'tmp' to the 'new' directory,\r
  76 > fsync on the 'new' directory for durability.\r
  77 > ---\r
  78 >  notmuch-insert.c | 39 ++++++++++++++++++++++++++++++++-------\r
  79 >  1 file changed, 32 insertions(+), 7 deletions(-)\r
  80 >\r
  81 > diff --git a/notmuch-insert.c b/notmuch-insert.c\r
  82 > index f09c579..831b322 100644\r
  83 > --- a/notmuch-insert.c\r
  84 > +++ b/notmuch-insert.c\r
  85 > @@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)\r
  86 >  /* Open a unique file in the Maildir 'tmp' directory.\r
  87 >   * Returns the file descriptor on success, or -1 on failure.\r
  88 >   * On success, file paths into the 'tmp' and 'new' directories are returned\r
  89 > - * via tmppath and newpath. */\r
  90 > + * via tmppath and newpath, and the path of the 'new' directory in newdir. */\r
  91 \r
  92 I found this comment very difficult to understand since it looks like\r
  93 the 'new' directory gets returned twice. Perhaps something like\r
  94 \r
  95 "On success, file paths for the message in the 'tmp' and 'new'\r
  96 directories are returned via tmppath and newpath, and the path of the\r
  97 'new' directory itself in newdir."\r
  98 \r
  99 \r
 100 Best wishes\r
 101 \r
 102 Mark\r
 103 \r
 104 >  static int\r
 105 >  maildir_open_tmp_file (void *ctx, const char *dir,\r
 106 > -                    char **tmppath, char **newpath)\r
 107 > +                    char **tmppath, char **newpath, char **newdir)\r
 108 >  {\r
 109 >      pid_t pid;\r
 110 >      char hostname[256];\r
 111 > @@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,\r
 112 >       return -1;\r
 113 >      }\r
 114 >  \r
 115 > +    *newdir = talloc_asprintf (ctx, "%s/new", dir);\r
 116 >      *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);\r
 117 > -    if (! *newpath) {\r
 118 > +    if (! *newdir || ! *newpath) {\r
 119 >       fprintf (stderr, "Out of memory\n");\r
 120 >       close (fd);\r
 121 >       unlink (*tmppath);\r
 122 > @@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,\r
 123 >   * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery\r
 124 >   */\r
 125 >  static notmuch_bool_t\r
 126 > -maildir_move_tmp_to_new (const char *tmppath, const char *newpath)\r
 127 > +maildir_move_tmp_to_new (const char *tmppath, const char *newpath,\r
 128 > +                      const char *newdir)\r
 129 >  {\r
 130 > +    notmuch_bool_t ret;\r
 131 > +    int fd;\r
 132 > +\r
 133 >      if (rename (tmppath, newpath) != 0) {\r
 134 >       fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));\r
 135 >       return FALSE;\r
 136 >      }\r
 137 >  \r
 138 > -    return TRUE;\r
 139 > +    /* Sync the 'new' directory after rename for durability. */\r
 140 > +    ret = TRUE;\r
 141 > +    fd = open (newdir, O_RDONLY);\r
 142 > +    if (fd == -1) {\r
 143 > +     fprintf (stderr, "Error: open() dir failed: %s\n", strerror (errno));\r
 144 > +     ret = FALSE;\r
 145 > +    }\r
 146 > +    if (ret && fsync (fd) != 0) {\r
 147 > +     fprintf (stderr, "Error: fsync() dir failed: %s\n", strerror (errno));\r
 148 > +     ret = FALSE;\r
 149 > +    }\r
 150 > +    if (fd != -1)\r
 151 > +     close (fd);\r
 152 > +    return ret;\r
 153 >  }\r
 154 >  \r
 155 >  /* Copy the contents of fdin into fdout. */\r
 156 > @@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const char *path,\r
 157 >  \r
 158 >      notmuch_message_thaw (message);\r
 159 >  \r
 160 > +    /* notmuch_message_tags_to_maildir_flags may rename the message file\r
 161 > +     * once more, and does so without fsyncing the directory afterwards.\r
 162 > +     * rename() is atomic so after a crash the file should appear under\r
 163 > +     * the old or new name. notmuch new should be able to rename the file\r
 164 > +     * again if required, so another fsync is not required, I think.\r
 165 > +     */\r
 166 >      notmuch_message_tags_to_maildir_flags (message);\r
 167 >  \r
 168 >      notmuch_message_destroy (message);\r
 169 > @@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,\r
 170 >  {\r
 171 >      char *tmppath;\r
 172 >      char *newpath;\r
 173 > +    char *newdir;\r
 174 >      int fdout;\r
 175 >      notmuch_bool_t ret;\r
 176 >  \r
 177 > -    fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath);\r
 178 > +    fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath, &newdir);\r
 179 >      if (fdout < 0) {\r
 180 >       return FALSE;\r
 181 >      }\r
 182 > @@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,\r
 183 >      }\r
 184 >      close (fdout);\r
 185 >      if (ret) {\r
 186 > -     ret = maildir_move_tmp_to_new (tmppath, newpath);\r
 187 > +     ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);\r
 188 >      }\r
 189 >      if (!ret) {\r
 190 >       unlink (tmppath);\r
 191 > -- \r
 192 > 1.7.12.1\r
 193 >\r
 194 > _______________________________________________\r
 195 > notmuch mailing list\r
 196 > notmuch@notmuchmail.org\r
 197 > http://notmuchmail.org/mailman/listinfo/notmuch\r