[PATCH 05/18] insert: move file from Maildir tmp to new

[Peter Wang]

On Sun, 18 Nov 2012 17:33:46 +, Mark Walters  
wrote:
> On Wed, 25 Jul 2012, Peter Wang  wrote:
> > Atomically move the new message file from the Maildir 'tmp' directory
> > to 'new'.
> > ---
> >  notmuch-insert.c |   18 ++
> >  1 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/notmuch-insert.c b/notmuch-insert.c
> > index 340f7e4..bab1fed 100644
> > --- a/notmuch-insert.c
> > +++ b/notmuch-insert.c
> > @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
> > **tmppath, char **newpath)
> >  }
> >  
> >  static notmuch_bool_t
> > +maildir_move_to_new (const char *tmppath, const char *newpath)
> > +{
> > +/* We follow the Dovecot recommendation to simply use rename()
> > + * instead of link() and unlink().
> > + */
> > +if (rename (tmppath, newpath) == 0) {
> > +   return TRUE;
> > +}
> 
> Do we want to overwrite an existing message with this name? As far as I
> can see rename does overwrite and link would not: was that why rename is
> better than link/unlink?
> 
> I would prefer not to overwrite but maybe there is a reason we need to. 
> Would a possible alternative be to loop when finding a tmp file until
> both the tmp file and the new file do not exist?

According to [1] it's all pointless -- just generate unique file names.

The dovecot maildir-save.c has this comment:

/* maildir spec says we should use link() + unlink() here. however
   since our filename is guaranteed to be unique, rename() works just
   as well, except faster. even if the filename wasn't unique, the
   problem could still happen if the file was already moved from
   new/ to cur/, so link() doesn't really provide any safety anyway.

   Besides the small temporary performance benefits, this rename() is
   almost required with OSX's HFS+ filesystem, since it implements
   hard links in a pretty ugly way, which makes the performance crawl
   when a lot of hard links are used. */

Well, that's one point of view.  I can't say I know any better.

Peter

[1]: http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery

[PATCH 05/18] insert: move file from Maildir tmp to new

[Mark Walters]

Hi 

On Mon, 19 Nov 2012, Peter Wang  wrote:
> On Sun, 18 Nov 2012 17:33:46 +, Mark Walters  gmail.com> wrote:
>> On Wed, 25 Jul 2012, Peter Wang  wrote:
>> > Atomically move the new message file from the Maildir 'tmp' directory
>> > to 'new'.
>> > ---
>> >  notmuch-insert.c |   18 ++
>> >  1 files changed, 18 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/notmuch-insert.c b/notmuch-insert.c
>> > index 340f7e4..bab1fed 100644
>> > --- a/notmuch-insert.c
>> > +++ b/notmuch-insert.c
>> > @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
>> > **tmppath, char **newpath)
>> >  }
>> >  
>> >  static notmuch_bool_t
>> > +maildir_move_to_new (const char *tmppath, const char *newpath)
>> > +{
>> > +/* We follow the Dovecot recommendation to simply use rename()
>> > + * instead of link() and unlink().
>> > + */
>> > +if (rename (tmppath, newpath) == 0) {
>> > +  return TRUE;
>> > +}
>> 
>> Do we want to overwrite an existing message with this name? As far as I
>> can see rename does overwrite and link would not: was that why rename is
>> better than link/unlink?
>> 
>> I would prefer not to overwrite but maybe there is a reason we need to. 
>> Would a possible alternative be to loop when finding a tmp file until
>> both the tmp file and the new file do not exist?
>
> According to [1] it's all pointless -- just generate unique file names.
>
> The dovecot maildir-save.c has this comment:
>
> /* maildir spec says we should use link() + unlink() here. however
>since our filename is guaranteed to be unique, rename() works just
>as well, except faster. even if the filename wasn't unique, the
>problem could still happen if the file was already moved from
>new/ to cur/, so link() doesn't really provide any safety anyway.
>
>Besides the small temporary performance benefits, this rename() is
>almost required with OSX's HFS+ filesystem, since it implements
>hard links in a pretty ugly way, which makes the performance crawl
>when a lot of hard links are used. */
>
> Well, that's one point of view.  I can't say I know any better.

I think I agree with you/them. Indeed, since files in cur can have
maildir flags we could find that this rename works but then the new file
gets stomped on by notmuch_message_tags_to_maildir_flags which also uses
rename. It might be work adding the link to [1] in the comment, but in
any case I am happy with this now.

Best wishes

Mark

>
> Peter
>
> [1]: http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery

Re: [PATCH 05/18] insert: move file from Maildir tmp to new

[Peter Wang]

On Sun, 18 Nov 2012 17:33:46 +, Mark Walters markwalters1...@gmail.com 
wrote:
 On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
  Atomically move the new message file from the Maildir 'tmp' directory
  to 'new'.
  ---
   notmuch-insert.c |   18 ++
   1 files changed, 18 insertions(+), 0 deletions(-)
 
  diff --git a/notmuch-insert.c b/notmuch-insert.c
  index 340f7e4..bab1fed 100644
  --- a/notmuch-insert.c
  +++ b/notmuch-insert.c
  @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
  **tmppath, char **newpath)
   }
   
   static notmuch_bool_t
  +maildir_move_to_new (const char *tmppath, const char *newpath)
  +{
  +/* We follow the Dovecot recommendation to simply use rename()
  + * instead of link() and unlink().
  + */
  +if (rename (tmppath, newpath) == 0) {
  +   return TRUE;
  +}
 
 Do we want to overwrite an existing message with this name? As far as I
 can see rename does overwrite and link would not: was that why rename is
 better than link/unlink?
 
 I would prefer not to overwrite but maybe there is a reason we need to. 
 Would a possible alternative be to loop when finding a tmp file until
 both the tmp file and the new file do not exist?

According to [1] it's all pointless -- just generate unique file names.

The dovecot maildir-save.c has this comment:

/* maildir spec says we should use link() + unlink() here. however
   since our filename is guaranteed to be unique, rename() works just
   as well, except faster. even if the filename wasn't unique, the
   problem could still happen if the file was already moved from
   new/ to cur/, so link() doesn't really provide any safety anyway.

   Besides the small temporary performance benefits, this rename() is
   almost required with OSX's HFS+ filesystem, since it implements
   hard links in a pretty ugly way, which makes the performance crawl
   when a lot of hard links are used. */

Well, that's one point of view.  I can't say I know any better.

Peter

[1]: http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 05/18] insert: move file from Maildir tmp to new

[Mark Walters]

On Wed, 25 Jul 2012, Peter Wang  wrote:
> Atomically move the new message file from the Maildir 'tmp' directory
> to 'new'.
> ---
>  notmuch-insert.c |   18 ++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 340f7e4..bab1fed 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
> **tmppath, char **newpath)
>  }
>  
>  static notmuch_bool_t
> +maildir_move_to_new (const char *tmppath, const char *newpath)
> +{
> +/* We follow the Dovecot recommendation to simply use rename()
> + * instead of link() and unlink().
> + */
> +if (rename (tmppath, newpath) == 0) {
> + return TRUE;
> +}

Do we want to overwrite an existing message with this name? As far as I
can see rename does overwrite and link would not: was that why rename is
better than link/unlink?

I would prefer not to overwrite but maybe there is a reason we need to. 
Would a possible alternative be to loop when finding a tmp file until
both the tmp file and the new file do not exist?

Best wishes

Mark


> +
> +fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
> +return FALSE;
> +}
> +
> +static notmuch_bool_t
>  copy_fd_data (int fdin, int fdout)
>  {
>  char buf[4096];
> @@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  
>  close (fdout);
>  
> +if (ret) {
> + ret = maildir_move_to_new (tmppath, newpath);
> +}
> +
>  if (!ret) {
>   unlink (tmppath);
>  }
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 05/18] insert: move file from Maildir tmp to new

[Mark Walters]

On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Atomically move the new message file from the Maildir 'tmp' directory
 to 'new'.
 ---
  notmuch-insert.c |   18 ++
  1 files changed, 18 insertions(+), 0 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index 340f7e4..bab1fed 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
 **tmppath, char **newpath)
  }
  
  static notmuch_bool_t
 +maildir_move_to_new (const char *tmppath, const char *newpath)
 +{
 +/* We follow the Dovecot recommendation to simply use rename()
 + * instead of link() and unlink().
 + */
 +if (rename (tmppath, newpath) == 0) {
 + return TRUE;
 +}

Do we want to overwrite an existing message with this name? As far as I
can see rename does overwrite and link would not: was that why rename is
better than link/unlink?

I would prefer not to overwrite but maybe there is a reason we need to. 
Would a possible alternative be to loop when finding a tmp file until
both the tmp file and the new file do not exist?

Best wishes

Mark


 +
 +fprintf (stderr, Error: rename() failed: %s\n, strerror (errno));
 +return FALSE;
 +}
 +
 +static notmuch_bool_t
  copy_fd_data (int fdin, int fdout)
  {
  char buf[4096];
 @@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  
  close (fdout);
  
 +if (ret) {
 + ret = maildir_move_to_new (tmppath, newpath);
 +}
 +
  if (!ret) {
   unlink (tmppath);
  }
 -- 
 1.7.4.4

 ___
 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 05/18] insert: move file from Maildir tmp to new

[Peter Wang]

Atomically move the new message file from the Maildir 'tmp' directory
to 'new'.
---
 notmuch-insert.c |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 340f7e4..bab1fed 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
**tmppath, char **newpath)
 }

 static notmuch_bool_t
+maildir_move_to_new (const char *tmppath, const char *newpath)
+{
+/* We follow the Dovecot recommendation to simply use rename()
+ * instead of link() and unlink().
+ */
+if (rename (tmppath, newpath) == 0) {
+   return TRUE;
+}
+
+fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
+return FALSE;
+}
+
+static notmuch_bool_t
 copy_fd_data (int fdin, int fdout)
 {
 char buf[4096];
@@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,

 close (fdout);

+if (ret) {
+   ret = maildir_move_to_new (tmppath, newpath);
+}
+
 if (!ret) {
unlink (tmppath);
 }
-- 
1.7.4.4

[PATCH 05/18] insert: move file from Maildir tmp to new

[Peter Wang]

Atomically move the new message file from the Maildir 'tmp' directory
to 'new'.
---
 notmuch-insert.c |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 340f7e4..bab1fed 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
**tmppath, char **newpath)
 }
 
 static notmuch_bool_t
+maildir_move_to_new (const char *tmppath, const char *newpath)
+{
+/* We follow the Dovecot recommendation to simply use rename()
+ * instead of link() and unlink().
+ */
+if (rename (tmppath, newpath) == 0) {
+   return TRUE;
+}
+
+fprintf (stderr, Error: rename() failed: %s\n, strerror (errno));
+return FALSE;
+}
+
+static notmuch_bool_t
 copy_fd_data (int fdin, int fdout)
 {
 char buf[4096];
@@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
 
 close (fdout);
 
+if (ret) {
+   ret = maildir_move_to_new (tmppath, newpath);
+}
+
 if (!ret) {
unlink (tmppath);
 }
-- 
1.7.4.4

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