Weird behaviour in notmuch new

Weird behaviour in notmuch new

[Mark Walters]

Hi

I was experimenting with letting notmuch new take an argument to tell it
to scan only a particular directory (and sub-directories) for new
messages. I came across the following strange behaviour which is also
present in master (with a fresh database)

I have a bunch of maildirs in /home/mail: so folders .mail.foo/
.mail.bar/ each of which has cur/new/tmp and all the messages are in
cur.

If I do mv .mail.foo .mail.bar/ and run notmuch new I get the expected
lots of renames (900 or so in the case I was trying). But if I then do
mv .mail.bar/.mail.foo . and run notmuch new almost all the messages get
removed (but 30 renames do get detected). If I then do touch .mail.foo/*
the messages get found again

I am guessing the 30 renames might be because those 30 have duplicates
somewhere else. 

But the other behaviour has me puzzled.

Best wishes

Mark

[PATCH] test: test folder renames

[Jani Nikula]

On Sun, 23 Feb 2014, Mark Walters <markwalters1009@gmail.com> wrote:
> I was experimenting with letting notmuch new take an argument to tell it
> to scan only a particular directory (and sub-directories) for new
> messages. I came across the following strange behaviour which is also
> present in master (with a fresh database)
>
> I have a bunch of maildirs in /home/mail: so folders .mail.foo/
> .mail.bar/ each of which has cur/new/tmp and all the messages are in
> cur.
>
> If I do mv .mail.foo .mail.bar/ and run notmuch new I get the expected
> lots of renames (900 or so in the case I was trying). But if I then do
> mv .mail.bar/.mail.foo . and run notmuch new almost all the messages get
> removed (but 30 renames do get detected). If I then do touch .mail.foo/*
> the messages get found again
>
> I am guessing the 30 renames might be because those 30 have duplicates
> somewhere else.
>
> But the other behaviour has me puzzled.

This test reproduces the problem for me, but it's not
deterministic. With the loop, I get roughly one fail per test run:

 FAIL   Rename folder back
 --- T051-new-renames.27.expected	2014-02-23 21:37:10.121774241 +0000
 +++ T051-new-renames.27.output		2014-02-23 21:37:10.121774241 +0000
 @@ -1 +1 @@
 -No new mail. Detected 10 file renames.
 +No new mail. Removed 10 messages.
 FAIL   Files remain the same
 --- T051-new-renames.28.expected	2014-02-23 21:37:10.133774652 +0000
 +++ T051-new-renames.28.output		2014-02-23 21:37:10.133774652 +0000
 @@ -1,13 +1,3 @@
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-121
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-122
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-123
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-124
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-125
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-126
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-127
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-128
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-129
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-130
  /path/to/test/tmp.T051-new-renames/mail/bar/msg-131
  /path/to/test/tmp.T051-new-renames/mail/bar/msg-132
  /path/to/test/tmp.T051-new-renames/mail/bar/msg-133

I'm as puzzled as you are.

BR,
Jani.
---
 test/T051-new-renames.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100755 test/T051-new-renames.sh

diff --git a/test/T051-new-renames.sh b/test/T051-new-renames.sh
new file mode 100755
index 000000000000..febe006f5888
--- /dev/null
+++ b/test/T051-new-renames.sh
@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+test_description='"notmuch new" with directory renames'
+. ./test-lib.sh
+
+for loop in `seq 10`; do
+
+rm -rf ${MAIL_DIR}
+
+for i in `seq 10`; do
+    generate_message '[dir]=foo' '[subject]="Message foo $i"'
+done
+
+for i in `seq 10`; do
+    generate_message '[dir]=bar' '[subject]="Message bar $i"'
+done
+
+test_begin_subtest "Index the messages, round $loop"
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "Added 20 new messages to the database."
+
+all_files=$(notmuch search --output=files \*)
+count_foo=$(notmuch count folder:foo)
+
+test_begin_subtest "Rename folder"
+mv ${MAIL_DIR}/foo ${MAIL_DIR}/baz
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
+
+test_begin_subtest "Rename folder back"
+mv ${MAIL_DIR}/baz ${MAIL_DIR}/foo
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
+
+test_begin_subtest "Files remain the same"
+output=$(notmuch search --output=files \*)
+test_expect_equal "$output" "$all_files"
+
+done
+
+test_done
-- 
1.8.5.3

Re: [PATCH] test: test folder renames

[Tomi Ollila]

On Sun, Feb 23 2014, Jani Nikula <jani@nikula.org> wrote:

> On Sun, 23 Feb 2014, Mark Walters <markwalters1009@gmail.com> wrote:
>> I was experimenting with letting notmuch new take an argument to tell it
>> to scan only a particular directory (and sub-directories) for new
>> messages. I came across the following strange behaviour which is also
>> present in master (with a fresh database)
>>
>> I have a bunch of maildirs in /home/mail: so folders .mail.foo/
>> .mail.bar/ each of which has cur/new/tmp and all the messages are in
>> cur.
>>
>> If I do mv .mail.foo .mail.bar/ and run notmuch new I get the expected
>> lots of renames (900 or so in the case I was trying). But if I then do
>> mv .mail.bar/.mail.foo . and run notmuch new almost all the messages get
>> removed (but 30 renames do get detected). If I then do touch .mail.foo/*
>> the messages get found again
>>
>> I am guessing the 30 renames might be because those 30 have duplicates
>> somewhere else.
>>
>> But the other behaviour has me puzzled.
>
> This test reproduces the problem for me, but it's not
> deterministic. With the loop, I get roughly one fail per test run:
>
>  FAIL   Rename folder back
>  --- T051-new-renames.27.expected	2014-02-23 21:37:10.121774241 +0000
>  +++ T051-new-renames.27.output		2014-02-23 21:37:10.121774241 +0000
>  @@ -1 +1 @@
>  -No new mail. Detected 10 file renames.
>  +No new mail. Removed 10 messages.
>  FAIL   Files remain the same
>  --- T051-new-renames.28.expected	2014-02-23 21:37:10.133774652 +0000
>  +++ T051-new-renames.28.output		2014-02-23 21:37:10.133774652 +0000
>  @@ -1,13 +1,3 @@
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-121
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-122
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-123
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-124
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-125
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-126
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-127
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-128
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-129
>  -/path/to/test/tmp.T051-new-renames/mail/foo/msg-130
>   /path/to/test/tmp.T051-new-renames/mail/bar/msg-131
>   /path/to/test/tmp.T051-new-renames/mail/bar/msg-132
>   /path/to/test/tmp.T051-new-renames/mail/bar/msg-133
>
> I'm as puzzled as you are.

find /path/to/test/tmp.T051-new-renames/ might show what is the order
or 'foo' and 'bar' directories there. the order might be arbitrary
-- it surely is not alphabetical and it might not be the order
created...

The order should not matter -- and maybe it didn't and some change
made that matter... 

I'd test now but I should be ZZZ :D

>
> BR,
> Jani.

Tomi

> ---
>  test/T051-new-renames.sh | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100755 test/T051-new-renames.sh
>
> diff --git a/test/T051-new-renames.sh b/test/T051-new-renames.sh
> new file mode 100755
> index 000000000000..febe006f5888
> --- /dev/null
> +++ b/test/T051-new-renames.sh
> @@ -0,0 +1,40 @@
> +#!/usr/bin/env bash
> +test_description='"notmuch new" with directory renames'
> +. ./test-lib.sh
> +
> +for loop in `seq 10`; do
> +
> +rm -rf ${MAIL_DIR}
> +
> +for i in `seq 10`; do
> +    generate_message '[dir]=foo' '[subject]="Message foo $i"'
> +done
> +
> +for i in `seq 10`; do
> +    generate_message '[dir]=bar' '[subject]="Message bar $i"'
> +done
> +
> +test_begin_subtest "Index the messages, round $loop"
> +output=$(NOTMUCH_NEW)
> +test_expect_equal "$output" "Added 20 new messages to the database."
> +
> +all_files=$(notmuch search --output=files \*)
> +count_foo=$(notmuch count folder:foo)
> +
> +test_begin_subtest "Rename folder"
> +mv ${MAIL_DIR}/foo ${MAIL_DIR}/baz
> +output=$(NOTMUCH_NEW)
> +test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
> +
> +test_begin_subtest "Rename folder back"
> +mv ${MAIL_DIR}/baz ${MAIL_DIR}/foo
> +output=$(NOTMUCH_NEW)
> +test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
> +
> +test_begin_subtest "Files remain the same"
> +output=$(notmuch search --output=files \*)
> +test_expect_equal "$output" "$all_files"
> +
> +done
> +
> +test_done
> -- 
> 1.8.5.3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/2] HACK: test: test folder renames

[Jani Nikula]

On Sun, 23 Feb 2014, Mark Walters <markwalters1009@gmail.com> wrote [1]:
> I was experimenting with letting notmuch new take an argument to tell it
> to scan only a particular directory (and sub-directories) for new
> messages. I came across the following strange behaviour which is also
> present in master (with a fresh database)
>
> I have a bunch of maildirs in /home/mail: so folders .mail.foo/
> .mail.bar/ each of which has cur/new/tmp and all the messages are in
> cur.
>
> If I do mv .mail.foo .mail.bar/ and run notmuch new I get the expected
> lots of renames (900 or so in the case I was trying). But if I then do
> mv .mail.bar/.mail.foo . and run notmuch new almost all the messages get
> removed (but 30 renames do get detected). If I then do touch .mail.foo/*
> the messages get found again
>
> I am guessing the 30 renames might be because those 30 have duplicates
> somewhere else.
>
> But the other behaviour has me puzzled.

This test reproduces the problem for me, but it's not deterministic,
and I've been unable to make it so. Thus there's a loop of 100
attempts, and usually I hit the problem several times like this:

 FAIL   Rename folder back
 --- T051-new-renames.27.expected	2014-02-23 21:37:10.121774241 +0000
 +++ T051-new-renames.27.output		2014-02-23 21:37:10.121774241 +0000
 @@ -1 +1 @@
 -No new mail. Detected 10 file renames.
 +No new mail. Removed 10 messages.
 FAIL   Files remain the same
 --- T051-new-renames.28.expected	2014-02-23 21:37:10.133774652 +0000
 +++ T051-new-renames.28.output		2014-02-23 21:37:10.133774652 +0000
 @@ -1,13 +1,3 @@
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-121
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-122
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-123
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-124
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-125
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-126
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-127
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-128
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-129
 -/path/to/test/tmp.T051-new-renames/mail/foo/msg-130
  /path/to/test/tmp.T051-new-renames/mail/bar/msg-131
  /path/to/test/tmp.T051-new-renames/mail/bar/msg-132
  /path/to/test/tmp.T051-new-renames/mail/bar/msg-133

This test is not suitable for merging since it's not
deterministic. But maybe it's good enough to assess the patch that
follows.

[1] id:87siray6th.fsf@qmul.ac.uk
---
 test/T051-new-renames.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100755 test/T051-new-renames.sh

diff --git a/test/T051-new-renames.sh b/test/T051-new-renames.sh
new file mode 100755
index 000000000000..3c1515da1085
--- /dev/null
+++ b/test/T051-new-renames.sh
@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+test_description='"notmuch new" with directory renames'
+. ./test-lib.sh
+
+for loop in `seq 100`; do
+
+rm -rf ${MAIL_DIR}
+
+for i in `seq 10`; do
+    generate_message '[dir]=foo' '[subject]="Message foo $i"'
+done
+
+for i in `seq 10`; do
+    generate_message '[dir]=bar' '[subject]="Message bar $i"'
+done
+
+test_begin_subtest "Index the messages, round $loop"
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "Added 20 new messages to the database."
+
+all_files=$(notmuch search --output=files \*)
+count_foo=$(notmuch count folder:foo)
+
+test_begin_subtest "Rename folder foo -> baz"
+mv ${MAIL_DIR}/foo ${MAIL_DIR}/baz
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
+
+test_begin_subtest "Rename folder back baz -> foo"
+mv ${MAIL_DIR}/baz ${MAIL_DIR}/foo
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
+
+test_begin_subtest "Files remain the same"
+output=$(notmuch search --output=files \*)
+test_expect_equal "$output" "$all_files"
+
+done
+
+test_done
-- 
2.1.4

[PATCH 2/2] cli: reset db directory mtime upon directory removal

[Jani Nikula]

The library does not have a function to remove a directory document
for a path. Usually this doesn't matter except for a slight waste of
space. However, if the same directory gets added to the filesystem
again, the old directory document is found with the old mtime. Reset
the directory mtime on removal to avoid problems.

The corner case that can hit this problem is renaming directories back
and forth. Renaming does not change the mtime of the directory in the
filesystem, and thus the old db directory document mtime may match the
fs mtime of the directory.

The long term fix might be to add a library function to remove a
directory document, however this is a much simpler and faster fix for
the time being.
---
 notmuch-new.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/notmuch-new.c b/notmuch-new.c
index 514e06a4d1f3..33645349cd5f 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -878,6 +878,15 @@ _remove_directory (void *ctx,
 	    goto DONE;
     }
 
+    /*
+     * XXX: The library does not have a function to remove a directory
+     * document for a path. Usually this doesn't matter except for a
+     * slight waste of space. However, if the directory gets added to
+     * the filesystem again, the old directory document is found with
+     * the old mtime. Reset the directory mtime to avoid problems.
+     */
+    notmuch_directory_set_mtime (directory, 0);
+
   DONE:
     notmuch_directory_destroy (directory);
     return status;
-- 
2.1.4

Re: [PATCH 2/2] cli: reset db directory mtime upon directory removal

[Tomi Ollila]

On Sat, Sep 05 2015, Jani Nikula <jani@nikula.org> wrote:

> The library does not have a function to remove a directory document
> for a path. Usually this doesn't matter except for a slight waste of
> space. However, if the same directory gets added to the filesystem
> again, the old directory document is found with the old mtime. Reset
> the directory mtime on removal to avoid problems.
>
> The corner case that can hit this problem is renaming directories back
> and forth. Renaming does not change the mtime of the directory in the
> filesystem, and thus the old db directory document mtime may match the
> fs mtime of the directory.
>
> The long term fix might be to add a library function to remove a
> directory document, however this is a much simpler and faster fix for
> the time being.
> ---

LGTM.

Tomi


>  notmuch-new.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 514e06a4d1f3..33645349cd5f 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -878,6 +878,15 @@ _remove_directory (void *ctx,
>  	    goto DONE;
>      }
>  
> +    /*
> +     * XXX: The library does not have a function to remove a directory
> +     * document for a path. Usually this doesn't matter except for a
> +     * slight waste of space. However, if the directory gets added to
> +     * the filesystem again, the old directory document is found with
> +     * the old mtime. Reset the directory mtime to avoid problems.
> +     */
> +    notmuch_directory_set_mtime (directory, 0);
> +
>    DONE:
>      notmuch_directory_destroy (directory);
>      return status;
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] cli: reset db directory mtime upon directory removal

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> The library does not have a function to remove a directory document
> for a path. Usually this doesn't matter except for a slight waste of
> space. However, if the same directory gets added to the filesystem
> again, the old directory document is found with the old mtime. Reset
> the directory mtime on removal to avoid problems.
>


pushed to master.

d

[PATCH] test: test folder renames

[David Bremner]

From: Jani Nikula <jani@nikula.org>

In [1] Mark Walters reported a problem with messages being removed
from the database when the parent directory was renamed. Jani Nikula
proposed [2] these tests but observed

   This test is not suitable for merging since it's not deterministic.

After applying Jani's patch [3], the tests now pass deterministically,
and could usefully act as regression tests.

[1]: id:87siray6th.fsf@qmul.ac.uk
[2]: id:1393191650-28333-1-git-send-email-jani@nikula.org
[3]: id:1441445731-4362-2-git-send-email-jani@nikula.org
---
 test/T051-new-renames.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100755 test/T051-new-renames.sh

diff --git a/test/T051-new-renames.sh b/test/T051-new-renames.sh
new file mode 100755
index 00000000..febe006f
--- /dev/null
+++ b/test/T051-new-renames.sh
@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+test_description='"notmuch new" with directory renames'
+. ./test-lib.sh
+
+for loop in `seq 10`; do
+
+rm -rf ${MAIL_DIR}
+
+for i in `seq 10`; do
+    generate_message '[dir]=foo' '[subject]="Message foo $i"'
+done
+
+for i in `seq 10`; do
+    generate_message '[dir]=bar' '[subject]="Message bar $i"'
+done
+
+test_begin_subtest "Index the messages, round $loop"
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "Added 20 new messages to the database."
+
+all_files=$(notmuch search --output=files \*)
+count_foo=$(notmuch count folder:foo)
+
+test_begin_subtest "Rename folder"
+mv ${MAIL_DIR}/foo ${MAIL_DIR}/baz
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
+
+test_begin_subtest "Rename folder back"
+mv ${MAIL_DIR}/baz ${MAIL_DIR}/foo
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
+
+test_begin_subtest "Files remain the same"
+output=$(notmuch search --output=files \*)
+test_expect_equal "$output" "$all_files"
+
+done
+
+test_done
-- 
2.34.1

Re: [PATCH] test: test folder renames

[Tomi Ollila]

On Sun, Dec 26 2021, David Bremner wrote:

> From: Jani Nikula <jani@nikula.org>
>
> In [1] Mark Walters reported a problem with messages being removed
> from the database when the parent directory was renamed. Jani Nikula
> proposed [2] these tests but observed

With that 'From:' line commit message starts to look weird...
>
>    This test is not suitable for merging since it's not deterministic.
>
> After applying Jani's patch [3], the tests now pass deterministically,
> and could usefully act as regression tests.
>
> [1]: id:87siray6th.fsf@qmul.ac.uk
> [2]: id:1393191650-28333-1-git-send-email-jani@nikula.org
> [3]: id:1441445731-4362-2-git-send-email-jani@nikula.org
> ---
>  test/T051-new-renames.sh | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100755 test/T051-new-renames.sh
>
> diff --git a/test/T051-new-renames.sh b/test/T051-new-renames.sh
> new file mode 100755
> index 00000000..febe006f
> --- /dev/null
> +++ b/test/T051-new-renames.sh
> @@ -0,0 +1,40 @@
> +#!/usr/bin/env bash
> +test_description='"notmuch new" with directory renames'
> +. ./test-lib.sh
> +
> +for loop in `seq 10`; do

{1..10}

> +
> +rm -rf ${MAIL_DIR}
> +
> +for i in `seq 10`; do

{1..10}

> +    generate_message '[dir]=foo' '[subject]="Message foo $i"'
> +done
> +
> +for i in `seq 10`; do

{1..10}

> +    generate_message '[dir]=bar' '[subject]="Message bar $i"'
> +done
> +
> +test_begin_subtest "Index the messages, round $loop"
> +output=$(NOTMUCH_NEW)
> +test_expect_equal "$output" "Added 20 new messages to the database."
> +
> +all_files=$(notmuch search --output=files \*)
> +count_foo=$(notmuch count folder:foo)
> +
> +test_begin_subtest "Rename folder"
> +mv ${MAIL_DIR}/foo ${MAIL_DIR}/baz
> +output=$(NOTMUCH_NEW)
> +test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
> +
> +test_begin_subtest "Rename folder back"
> +mv ${MAIL_DIR}/baz ${MAIL_DIR}/foo
> +output=$(NOTMUCH_NEW)
> +test_expect_equal "$output" "No new mail. Detected $count_foo file renames."
> +
> +test_begin_subtest "Files remain the same"
> +output=$(notmuch search --output=files \*)
> +test_expect_equal "$output" "$all_files"
> +
> +done
> +
> +test_done
> -- 
> 2.34.1

Re: [PATCH] test: test folder renames

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sun, Dec 26 2021, David Bremner wrote:
>
>> From: Jani Nikula <jani@nikula.org>
>>
>> In [1] Mark Walters reported a problem with messages being removed
>> from the database when the parent directory was renamed. Jani Nikula
>> proposed [2] these tests but observed
>
> With that 'From:' line commit message starts to look weird...

thanks for the review. I applied your suggestions (and also fixed the
inclusion of test-lib.sh to match the other tests), and applied to
master.

One more test to break 1800 tests. I guess that must be a good thing?

d