Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[David Bremner]

Jani Nikula  writes:

> +/* Exit status code indicating that file(s) in the mail store were
> + * removed or renamed after notmuch new scanned the directories but
> + * before indexing the file(s). If the file was renamed, the indexing
> + * might not be complete, and the user is advised to re-run notmuch
> + * new.
> + */
> +#define NOTMUCH_EXIT_VANISHED_FILES 10
> +

What do you think about defining something like NOTMUCH_EXIT_TEMPFAIL 75
(to match EX_TEMPFAIL) and using that?  There is also some stalled patch
around for insert to use EX_TEMPFAIL (although in that case part of the
reason it has stalled is I'm not convinced the error is temporary).

I think such an exit code would also make sense for locking failures;
but that is a different discussion.

Other than that the patch looks like an incremental improvement
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[David Bremner]

Brian Sniffen  writes:

> That's hard, given dovecot pointed at the same maildir: it quickly
> moves files from new to cur. That makes notmuch insert pretty useless,
> and I rely on notmuch new to approach correctness.

I don't think this discussion is related to notmuch insert at all. If
you have found a race condition (or some other concurrency issue) in
notmuch-insert please report that seperately.

>
> But maybe I misunderstand: is the idea that it will return an error
>but keep processing?  Or stop on that error?

The whole discussion started because under certain circumstances it will
stop processing. The proposed patch makes it continue processing, but
report an error at the end.

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

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[Brian Sniffen]

> On Nov 12, 2016, at 11:10 AM, David Bremner  wrote:
> 
> Brian Sniffen  writes:
> 
>>> 
>>> OK, but the patch proposed works both for people who want to be notified
>>> of this problem, and those that don't (with appropriate shell wrapping
>>> checking the return code).  
>> 
>> I think it will loop; how do I guarantee termination and indexing of all 
>> present messages if deletions cause errors?
> 
> stop deleting things? You can't guarantee termination and indexing of
> all present messages by ignoring deletions either.

That's hard, given dovecot pointed at the same maildir: it quickly moves files 
from new to cur. That makes notmuch insert pretty useless, and I rely on 
notmuch new to approach correctness. 

But maybe I misunderstand: is the idea that it will return an error but keep 
processing?  Or stop on that error?



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

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[Jani Nikula]

On Sat, 12 Nov 2016, Brian Sniffen  wrote:
>> 
>> OK, but the patch proposed works both for people who want to be notified
>> of this problem, and those that don't (with appropriate shell wrapping
>> checking the return code).  
>
> I think it will loop; how do I guarantee termination and indexing of
> all present messages if deletions cause errors?

Please note that we're talking about deletions and renames *between* the
scandir(3) call and going through the results it returns, during a
single invocation of 'notmuch new'. On the next run, scandir(3) won't
return the entry, and we'll think it's gone.

(Of course, if you keep deleting/renaming files and running 'notmuch
new' simultaneously all the time, you'll hit this on some other files on
the consequent runs, but then you asked for it...)

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[David Bremner]

David Bremner  writes:

> Brian Sniffen  writes:
>
>>> 
>>> OK, but the patch proposed works both for people who want to be notified
>>> of this problem, and those that don't (with appropriate shell wrapping
>>> checking the return code).  
>>
>> I think it will loop; how do I guarantee termination and indexing of all 
>> present messages if deletions cause errors?
>>
>> -Brian
>
> stop deleting things? You can't guarantee termination and indexing of
> all present messages by ignoring deletions either.
>
> d

Sorry, that was written in haste. Of course if that's your goal ignoring
deletions is ok, but renames will still get you, and we have no way of
knowing the difference.  In any case, I was more thinking that people
who want to ignore deletions could check for the specific error code and
consider that not-an-error.

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

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[Brian Sniffen]

> 
> OK, but the patch proposed works both for people who want to be notified
> of this problem, and those that don't (with appropriate shell wrapping
> checking the return code).  

I think it will loop; how do I guarantee termination and indexing of all 
present messages if deletions cause errors?

-Brian
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[David Bremner]

Brian Sniffen  writes:

>> 
>> OK, but the patch proposed works both for people who want to be notified
>> of this problem, and those that don't (with appropriate shell wrapping
>> checking the return code).  
>
> I think it will loop; how do I guarantee termination and indexing of all 
> present messages if deletions cause errors?
>
> -Brian

stop deleting things? You can't guarantee termination and indexing of
all present messages by ignoring deletions either.

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

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[David Bremner]

Paul Wise  writes:

> On Sat, 2016-11-05 at 14:57 +0200, Jani Nikula wrote:
>
>> Add a new exit code for when files vanished, so the caller has a
>> chance to detect the race and re-run notmuch new to recover.
>
> I don't think this is the right approach for two reasons:
>
> The exit code you have chosen is still a failure so I will still get
> notified for a minor issue. I use chronic to detect fail scenarios.
>
> This is a pretty normal scenario when you have a mail program open and
> are auto-running `notmuch new` on a scheduled basis or when new mail
> arrives. notmuch should just ignore the error and continue as normal.
>

OK, but the patch proposed works both for people who want to be notified
of this problem, and those that don't (with appropriate shell wrapping
checking the return code).  That seems better than hiding it for
everyone. And certainly an improvement on the status quo. A possible
future enhancement would be a flag like notmuch insert has to control
the treatment of these errors.

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

Re: [PATCH] cli: consider files vanishing during notmuch new non-fatal

[Paul Wise]

On Sat, 2016-11-05 at 14:57 +0200, Jani Nikula wrote:

> Add a new exit code for when files vanished, so the caller has a
> chance to detect the race and re-run notmuch new to recover.

I don't think this is the right approach for two reasons:

The exit code you have chosen is still a failure so I will still get
notified for a minor issue. I use chronic to detect fail scenarios.

This is a pretty normal scenario when you have a mail program open and
are auto-running `notmuch new` on a scheduled basis or when new mail
arrives. notmuch should just ignore the error and continue as normal.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] cli: consider files vanishing during notmuch new non-fatal

[Jani Nikula]

If some software other than notmuch new renames or removes files
during the notmuch new scan (specifically after scandir but before
indexing the file), keep going instead of bailing out. Failing to
index the file is just a race condition between notmuch and the other
software; the rename could happen after the notmuch new scan
anyway. It's not fatal, and we'll catch the renamed files on the next
scan.

Add a new exit code for when files vanished, so the caller has a
chance to detect the race and re-run notmuch new to recover.

Reported by Paul Wise  at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=843127

---

Having notmuch new re-run (parts of) the scan automatically seems a
rather more involved change. So does inotify support. This simple
change both finishes the scan and lets the user recover, IMO a
reasonable first step.

Please suggest a better alternative to "vanish" in code...
---
 notmuch-client.h |  8 
 notmuch-new.c| 15 ---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 9ce2aef17431..d2057e26c5cd 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -114,6 +114,14 @@ chomp_newline (char *str)
str[strlen(str)-1] = '\0';
 }
 
+/* Exit status code indicating that file(s) in the mail store were
+ * removed or renamed after notmuch new scanned the directories but
+ * before indexing the file(s). If the file was renamed, the indexing
+ * might not be complete, and the user is advised to re-run notmuch
+ * new.
+ */
+#define NOTMUCH_EXIT_VANISHED_FILES 10
+
 /* Exit status code indicating the requested format version is too old
  * (support for that version has been dropped).  CLI code should use
  * notmuch_exit_if_unsupported_format rather than directly exiting
diff --git a/notmuch-new.c b/notmuch-new.c
index c55dea7bc1b7..e694a6adcee1 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -53,6 +53,7 @@ typedef struct {
 int total_files;
 int processed_files;
 int added_messages, removed_messages, renamed_messages;
+int vanished_files;
 struct timeval tv_start;
 
 _filename_list_t *removed_files;
@@ -280,11 +281,13 @@ add_file (notmuch_database_t *notmuch, const char 
*filename,
 case NOTMUCH_STATUS_FILE_NOT_EMAIL:
fprintf (stderr, "Note: Ignoring non-mail file: %s\n", filename);
break;
-/* Fatal issues. Don't process anymore. */
 case NOTMUCH_STATUS_FILE_ERROR:
+   /* Someone renamed/removed the file between scandir and now. */
+   state->vanished_files++;
fprintf (stderr, "Unexpected error with file %s\n", filename);
(void) print_status_database ("add_file", notmuch, status);
-   goto DONE;
+   break;
+/* Fatal issues. Don't process anymore. */
 case NOTMUCH_STATUS_READ_ONLY_DATABASE:
 case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
 case NOTMUCH_STATUS_OUT_OF_MEMORY:
@@ -1151,5 +1154,11 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
char *argv[])
 if (!no_hooks && !ret && !interrupted)
ret = notmuch_run_hook (db_path, "post-new");
 
-return ret || interrupted ? EXIT_FAILURE : EXIT_SUCCESS;
+if (ret || interrupted)
+   return EXIT_FAILURE;
+
+if (add_files_state.vanished_files)
+   return NOTMUCH_EXIT_VANISHED_FILES;
+
+return EXIT_SUCCESS;
 }
-- 
2.1.4

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