subject:"\[PATCH v2 15\/20\] insert\: fsync new directory after rename"

[PATCH v2 15/20] insert: fsync new directory after rename

2012-11-25 Thread Mark Walters

On Sun, 25 Nov 2012, Peter Wang  wrote:
> After moving the file from the 'tmp' to the 'new' directory,
> fsync on the 'new' directory for durability.
> ---
>  notmuch-insert.c | 39 ---
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index f09c579..831b322 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
>  /* Open a unique file in the Maildir 'tmp' directory.
>   * Returns the file descriptor on success, or -1 on failure.
>   * On success, file paths into the 'tmp' and 'new' directories are returned
> - * via tmppath and newpath. */
> + * via tmppath and newpath, and the path of the 'new' directory in newdir. */

I found this comment very difficult to understand since it looks like
the 'new' directory gets returned twice. Perhaps something like

"On success, file paths for the message in the 'tmp' and 'new'
directories are returned via tmppath and newpath, and the path of the
'new' directory itself in newdir."


Best wishes

Mark

>  static int
>  maildir_open_tmp_file (void *ctx, const char *dir,
> -char **tmppath, char **newpath)
> +char **tmppath, char **newpath, char **newdir)
>  {
>  pid_t pid;
>  char hostname[256];
> @@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
>   return -1;
>  }
>  
> +*newdir = talloc_asprintf (ctx, "%s/new", dir);
>  *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
> -if (! *newpath) {
> +if (! *newdir || ! *newpath) {
>   fprintf (stderr, "Out of memory\n");
>   close (fd);
>   unlink (*tmppath);
> @@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
>   * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
>   */
>  static notmuch_bool_t
> -maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
> +maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
> +  const char *newdir)
>  {
> +notmuch_bool_t ret;
> +int fd;
> +
>  if (rename (tmppath, newpath) != 0) {
>   fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
>   return FALSE;
>  }
>  
> -return TRUE;
> +/* Sync the 'new' directory after rename for durability. */
> +ret = TRUE;
> +fd = open (newdir, O_RDONLY);
> +if (fd == -1) {
> + fprintf (stderr, "Error: open() dir failed: %s\n", strerror (errno));
> + ret = FALSE;
> +}
> +if (ret && fsync (fd) != 0) {
> + fprintf (stderr, "Error: fsync() dir failed: %s\n", strerror (errno));
> + ret = FALSE;
> +}
> +if (fd != -1)
> + close (fd);
> +return ret;
>  }
>  
>  /* Copy the contents of fdin into fdout. */
> @@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
> char *path,
>  
>  notmuch_message_thaw (message);
>  
> +/* notmuch_message_tags_to_maildir_flags may rename the message file
> + * once more, and does so without fsyncing the directory afterwards.
> + * rename() is atomic so after a crash the file should appear under
> + * the old or new name. notmuch new should be able to rename the file
> + * again if required, so another fsync is not required, I think.
> + */
>  notmuch_message_tags_to_maildir_flags (message);
>  
>  notmuch_message_destroy (message);
> @@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  {
>  char *tmppath;
>  char *newpath;
> +char *newdir;
>  int fdout;
>  notmuch_bool_t ret;
>  
> -fdout = maildir_open_tmp_file (ctx, dir, , );
> +fdout = maildir_open_tmp_file (ctx, dir, , , );
>  if (fdout < 0) {
>   return FALSE;
>  }
> @@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  }
>  close (fdout);
>  if (ret) {
> - ret = maildir_move_tmp_to_new (tmppath, newpath);
> + ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
>  }
>  if (!ret) {
>   unlink (tmppath);
> -- 
> 1.7.12.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 15/20] insert: fsync new directory after rename

2012-11-25 Thread Peter Wang

After moving the file from the 'tmp' to the 'new' directory,
fsync on the 'new' directory for durability.
---
 notmuch-insert.c | 39 ---
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index f09c579..831b322 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
 /* Open a unique file in the Maildir 'tmp' directory.
  * Returns the file descriptor on success, or -1 on failure.
  * On success, file paths into the 'tmp' and 'new' directories are returned
- * via tmppath and newpath. */
+ * via tmppath and newpath, and the path of the 'new' directory in newdir. */
 static int
 maildir_open_tmp_file (void *ctx, const char *dir,
-  char **tmppath, char **newpath)
+  char **tmppath, char **newpath, char **newdir)
 {
 pid_t pid;
 char hostname[256];
@@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
return -1;
 }

+*newdir = talloc_asprintf (ctx, "%s/new", dir);
 *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
-if (! *newpath) {
+if (! *newdir || ! *newpath) {
fprintf (stderr, "Out of memory\n");
close (fd);
unlink (*tmppath);
@@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
  * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
  */
 static notmuch_bool_t
-maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
+maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
+const char *newdir)
 {
+notmuch_bool_t ret;
+int fd;
+
 if (rename (tmppath, newpath) != 0) {
fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
return FALSE;
 }

-return TRUE;
+/* Sync the 'new' directory after rename for durability. */
+ret = TRUE;
+fd = open (newdir, O_RDONLY);
+if (fd == -1) {
+   fprintf (stderr, "Error: open() dir failed: %s\n", strerror (errno));
+   ret = FALSE;
+}
+if (ret && fsync (fd) != 0) {
+   fprintf (stderr, "Error: fsync() dir failed: %s\n", strerror (errno));
+   ret = FALSE;
+}
+if (fd != -1)
+   close (fd);
+return ret;
 }

 /* Copy the contents of fdin into fdout. */
@@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path,

 notmuch_message_thaw (message);

+/* notmuch_message_tags_to_maildir_flags may rename the message file
+ * once more, and does so without fsyncing the directory afterwards.
+ * rename() is atomic so after a crash the file should appear under
+ * the old or new name. notmuch new should be able to rename the file
+ * again if required, so another fsync is not required, I think.
+ */
 notmuch_message_tags_to_maildir_flags (message);

 notmuch_message_destroy (message);
@@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
 {
 char *tmppath;
 char *newpath;
+char *newdir;
 int fdout;
 notmuch_bool_t ret;

-fdout = maildir_open_tmp_file (ctx, dir, , );
+fdout = maildir_open_tmp_file (ctx, dir, , , );
 if (fdout < 0) {
return FALSE;
 }
@@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int 
fdin,
 }
 close (fdout);
 if (ret) {
-   ret = maildir_move_tmp_to_new (tmppath, newpath);
+   ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
 }
 if (!ret) {
unlink (tmppath);
-- 
1.7.12.1

Re: [PATCH v2 15/20] insert: fsync new directory after rename

2012-11-25 Thread Mark Walters

On Sun, 25 Nov 2012, Peter Wang noval...@gmail.com wrote:
 After moving the file from the 'tmp' to the 'new' directory,
 fsync on the 'new' directory for durability.
 ---
  notmuch-insert.c | 39 ---
  1 file changed, 32 insertions(+), 7 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index f09c579..831b322 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
  /* Open a unique file in the Maildir 'tmp' directory.
   * Returns the file descriptor on success, or -1 on failure.
   * On success, file paths into the 'tmp' and 'new' directories are returned
 - * via tmppath and newpath. */
 + * via tmppath and newpath, and the path of the 'new' directory in newdir. */

I found this comment very difficult to understand since it looks like
the 'new' directory gets returned twice. Perhaps something like

On success, file paths for the message in the 'tmp' and 'new'
directories are returned via tmppath and newpath, and the path of the
'new' directory itself in newdir.


Best wishes

Mark

  static int
  maildir_open_tmp_file (void *ctx, const char *dir,
 -char **tmppath, char **newpath)
 +char **tmppath, char **newpath, char **newdir)
  {
  pid_t pid;
  char hostname[256];
 @@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
   return -1;
  }
  
 +*newdir = talloc_asprintf (ctx, %s/new, dir);
  *newpath = talloc_asprintf (ctx, %s/new/%s, dir, filename);
 -if (! *newpath) {
 +if (! *newdir || ! *newpath) {
   fprintf (stderr, Out of memory\n);
   close (fd);
   unlink (*tmppath);
 @@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
   * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
   */
  static notmuch_bool_t
 -maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
 +maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
 +  const char *newdir)
  {
 +notmuch_bool_t ret;
 +int fd;
 +
  if (rename (tmppath, newpath) != 0) {
   fprintf (stderr, Error: rename() failed: %s\n, strerror (errno));
   return FALSE;
  }
  
 -return TRUE;
 +/* Sync the 'new' directory after rename for durability. */
 +ret = TRUE;
 +fd = open (newdir, O_RDONLY);
 +if (fd == -1) {
 + fprintf (stderr, Error: open() dir failed: %s\n, strerror (errno));
 + ret = FALSE;
 +}
 +if (ret  fsync (fd) != 0) {
 + fprintf (stderr, Error: fsync() dir failed: %s\n, strerror (errno));
 + ret = FALSE;
 +}
 +if (fd != -1)
 + close (fd);
 +return ret;
  }
  
  /* Copy the contents of fdin into fdout. */
 @@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
 char *path,
  
  notmuch_message_thaw (message);
  
 +/* notmuch_message_tags_to_maildir_flags may rename the message file
 + * once more, and does so without fsyncing the directory afterwards.
 + * rename() is atomic so after a crash the file should appear under
 + * the old or new name. notmuch new should be able to rename the file
 + * again if required, so another fsync is not required, I think.
 + */
  notmuch_message_tags_to_maildir_flags (message);
  
  notmuch_message_destroy (message);
 @@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  {
  char *tmppath;
  char *newpath;
 +char *newdir;
  int fdout;
  notmuch_bool_t ret;
  
 -fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath);
 +fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath, newdir);
  if (fdout  0) {
   return FALSE;
  }
 @@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  }
  close (fdout);
  if (ret) {
 - ret = maildir_move_tmp_to_new (tmppath, newpath);
 + ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
  }
  if (!ret) {
   unlink (tmppath);
 -- 
 1.7.12.1

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 15/20] insert: fsync new directory after rename

2012-11-24 Thread Peter Wang

After moving the file from the 'tmp' to the 'new' directory,
fsync on the 'new' directory for durability.
---
 notmuch-insert.c | 39 ---
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index f09c579..831b322 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
 /* Open a unique file in the Maildir 'tmp' directory.
  * Returns the file descriptor on success, or -1 on failure.
  * On success, file paths into the 'tmp' and 'new' directories are returned
- * via tmppath and newpath. */
+ * via tmppath and newpath, and the path of the 'new' directory in newdir. */
 static int
 maildir_open_tmp_file (void *ctx, const char *dir,
-  char **tmppath, char **newpath)
+  char **tmppath, char **newpath, char **newdir)
 {
 pid_t pid;
 char hostname[256];
@@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
return -1;
 }
 
+*newdir = talloc_asprintf (ctx, %s/new, dir);
 *newpath = talloc_asprintf (ctx, %s/new/%s, dir, filename);
-if (! *newpath) {
+if (! *newdir || ! *newpath) {
fprintf (stderr, Out of memory\n);
close (fd);
unlink (*tmppath);
@@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
  * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
  */
 static notmuch_bool_t
-maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
+maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
+const char *newdir)
 {
+notmuch_bool_t ret;
+int fd;
+
 if (rename (tmppath, newpath) != 0) {
fprintf (stderr, Error: rename() failed: %s\n, strerror (errno));
return FALSE;
 }
 
-return TRUE;
+/* Sync the 'new' directory after rename for durability. */
+ret = TRUE;
+fd = open (newdir, O_RDONLY);
+if (fd == -1) {
+   fprintf (stderr, Error: open() dir failed: %s\n, strerror (errno));
+   ret = FALSE;
+}
+if (ret  fsync (fd) != 0) {
+   fprintf (stderr, Error: fsync() dir failed: %s\n, strerror (errno));
+   ret = FALSE;
+}
+if (fd != -1)
+   close (fd);
+return ret;
 }
 
 /* Copy the contents of fdin into fdout. */
@@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path,
 
 notmuch_message_thaw (message);
 
+/* notmuch_message_tags_to_maildir_flags may rename the message file
+ * once more, and does so without fsyncing the directory afterwards.
+ * rename() is atomic so after a crash the file should appear under
+ * the old or new name. notmuch new should be able to rename the file
+ * again if required, so another fsync is not required, I think.
+ */
 notmuch_message_tags_to_maildir_flags (message);
 
 notmuch_message_destroy (message);
@@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
 {
 char *tmppath;
 char *newpath;
+char *newdir;
 int fdout;
 notmuch_bool_t ret;
 
-fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath);
+fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath, newdir);
 if (fdout  0) {
return FALSE;
 }
@@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int 
fdin,
 }
 close (fdout);
 if (ret) {
-   ret = maildir_move_tmp_to_new (tmppath, newpath);
+   ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
 }
 if (!ret) {
unlink (tmppath);
-- 
1.7.12.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 15/20] insert: fsync new directory after rename

[PATCH v2 15/20] insert: fsync new directory after rename

Re: [PATCH v2 15/20] insert: fsync new directory after rename

[PATCH v2 15/20] insert: fsync new directory after rename

4 matches

Site Navigation

Mail list logo

Footer information