[PATCH v3 2/7] test thread breakage when messages are removed and re-added

diff --git a/5c/9854f08b602f0a1d1cbe94b45cffa4d75358b9 b/5c/9854f08b602f0a1d1cbe94b45cffa4d75358b9
new file mode 100644 (file)
index 0000000..e00c433
--- /dev/null
+++ b/5c/9854f08b602f0a1d1cbe94b45cffa4d75358b9
@@ -0,0 +1,274 @@
+Return-Path: <dkg@fifthhorseman.net>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by arlo.cworth.org (Postfix) with ESMTP id E0F3F6DE02DD\r
+ for <notmuch@notmuchmail.org>; Fri,  8 Apr 2016 18:02:46 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at cworth.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 0\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
+ autolearn=disabled\r
+Received: from arlo.cworth.org ([127.0.0.1])\r
+ by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id 6tE6w4f95HQZ for <notmuch@notmuchmail.org>;\r
+ Fri,  8 Apr 2016 18:02:39 -0700 (PDT)\r
+Received: from che.mayfirst.org (che.mayfirst.org [209.234.253.108])\r
+ by arlo.cworth.org (Postfix) with ESMTP id C349A6DE028C\r
+ for <notmuch@notmuchmail.org>; Fri,  8 Apr 2016 18:02:38 -0700 (PDT)\r
+Received: from fifthhorseman.net (unknown [201.140.212.132])\r
+ by che.mayfirst.org (Postfix) with ESMTPSA id 969C110083\r
+ for <notmuch@notmuchmail.org>; Fri,  8 Apr 2016 21:02:36 -0400 (EDT)\r
+Received: by fifthhorseman.net (Postfix, from userid 1000)\r
+ id F13151FE7C; Fri,  8 Apr 2016 22:02:34 -0300 (ART)\r
+From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>\r
+To: Notmuch Mail <notmuch@notmuchmail.org>\r
+Subject: [PATCH v3 2/7] test thread breakage when messages are removed and\r
+ re-added\r
+Date: Fri,  8 Apr 2016 22:02:29 -0300\r
+Message-Id: <1460163754-22994-2-git-send-email-dkg@fifthhorseman.net>\r
+X-Mailer: git-send-email 2.8.0.rc3\r
+In-Reply-To: <1460163754-22994-1-git-send-email-dkg@fifthhorseman.net>\r
+References: <1459445693-3900-1-git-send-email-dkg@fifthhorseman.net>\r
+ <1460163754-22994-1-git-send-email-dkg@fifthhorseman.net>\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.20\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <https://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch/>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <https://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Sat, 09 Apr 2016 01:02:47 -0000\r
+\r
+This test (T590-thread-breakage.sh) has known-broken subtests.\r
+\r
+If you have a two-message thread where message "B" is in-reply-to "A",\r
+notmuch rightly sees this as a single thread.\r
+\r
+But if you:\r
+\r
+ * remove "A" from the message store\r
+ * run "notmuch new"\r
+ * add "A" back into the message store\r
+ * re-run "notmuch new"\r
+\r
+Then notmuch sees the messages as distinct threads.\r
+\r
+This happens because if you insert "B" initially (before anything is\r
+known about "A"), then a "ghost message" gets added to the database in\r
+reference to "A" that is in the same thread, which "A" takes over when\r
+it appears.\r
+\r
+But if "A" is subsequently removed, no ghost message is retained, so\r
+when "A" appears, it is treated as a new thread.\r
+\r
+I see a few options to fix this:\r
+\r
+ghost-on-removal\r
+----------------\r
+\r
+We could unilaterally add a ghost upon message removal.  This has a\r
+few disadvantages: the message index would leak information about what\r
+messages the user has ever been exposed to, and we also create a\r
+perpetually-growing dataset -- the ghosts can never be removed.\r
+\r
+ghost-on-removal-when-shared-thread-exists\r
+------------------------------------------\r
+\r
+We could add a ghost upon message removal iff there are other\r
+non-ghost messages with the same thread ID.\r
+\r
+We'd also need to remove all ghost messages that share a thread when\r
+the last non-ghost message in that thread is removed.\r
+\r
+This still has a bit of information leakage, though: the message index\r
+would reveal that i've seen a newer message in a thread, even if i had\r
+deleted it from my message store\r
+\r
+track-dependencies\r
+------------------\r
+\r
+rather than a simple "ghost-message" we could store all the (A,B)\r
+message-reference pairs internally, showing which messages A reference\r
+which other messages B.\r
+\r
+Then removal of message X would require deleting all message-reference\r
+pairs (X,B), and only deleting a ghost message if no (A,X) reference\r
+pair exists.\r
+\r
+This requires modifying the database by adding a new and fairly weird\r
+table that would need to be indexed by both columns.  I don't know\r
+whether xapian has nice ways to do that.\r
+\r
+scan-dependencies\r
+-----------------\r
+\r
+Without modifying the database, we could do something less efficient.\r
+\r
+Upon removal of message X, we could scan the headers of all non-ghost\r
+messages that share a thread with X.  If any of those messages refers\r
+to X, we would add a ghost message.  If none of them do, then we would\r
+just drop X entirely from the table.\r
+\r
+---------------------\r
+\r
+One risk of attempted fixes to this problem is that we could fail to\r
+remove the search term indexes entirely.  This test contains\r
+additional subtests to guard against that.\r
+\r
+This test also ensures that the right number of ghost messages exist\r
+in each situation; this will help us ensure we don't accumulate ghosts\r
+indefinitely or leak too much information about what messages we've\r
+seen or not seen, while still making it easy to reassemble threads\r
+when messages come in out-of-order.\r
+---\r
+ test/T590-thread-breakage.sh | 131 +++++++++++++++++++++++++++++++++++++++++++\r
+ 1 file changed, 131 insertions(+)\r
+ create mode 100755 test/T590-thread-breakage.sh\r
+\r
+diff --git a/test/T590-thread-breakage.sh b/test/T590-thread-breakage.sh\r
+new file mode 100755\r
+index 0000000..24ffb42\r
+--- /dev/null\r
++++ b/test/T590-thread-breakage.sh\r
+@@ -0,0 +1,131 @@\r
++#!/usr/bin/env bash\r
++#\r
++# Copyright (c) 2016 Daniel Kahn Gillmor\r
++#\r
++\r
++test_description='thread breakage during reindexing\r
++\r
++notmuch uses ghost documents to track messages we have seen references\r
++to but have never seen.  Regardless of the order of delivery, message\r
++deletion, and reindexing, the list of ghost messages for a given\r
++stored corpus should not vary, so that threads can be reassmebled\r
++cleanly.\r
++\r
++In practice, we accept a small amount of variation (and therefore\r
++traffic pattern metadata leakage to be stored in the index) for the\r
++sake of efficiency.\r
++\r
++This test also embeds some subtests to ensure that indexing actually\r
++works properly and attempted fixes to threading issues do not break\r
++the expected contents of the index.'\r
++\r
++. ./test-lib.sh || exit 1\r
++\r
++message_a() {\r
++    mkdir -p ${MAIL_DIR}/cur\r
++    cat > ${MAIL_DIR}/cur/a <<EOF\r
++Subject: First message\r
++Message-ID: <a@example.net>\r
++From: Alice <alice@example.net>\r
++To: Bob <bob@example.net>\r
++Date: Thu, 31 Mar 2016 20:10:00 -0400\r
++\r
++This is the first message in the thread.\r
++Apple\r
++EOF\r
++}\r
++\r
++message_b() {\r
++    mkdir -p ${MAIL_DIR}/cur\r
++    cat > ${MAIL_DIR}/cur/b <<EOF\r
++Subject: Second message\r
++Message-ID: <b@example.net>\r
++In-Reply-To: <a@example.net>\r
++References: <a@example.net>\r
++From: Bob <bob@example.net>\r
++To: Alice <alice@example.net>\r
++Date: Thu, 31 Mar 2016 20:15:00 -0400\r
++\r
++This is the second message in the thread.\r
++Banana\r
++EOF\r
++}\r
++\r
++\r
++test_subject_count() {\r
++    test_begin_subtest "${3:-looking for $2 instance of '$1'}"\r
++    count=$(notmuch count --output=threads "$1")\r
++    test_expect_equal "$count" "$2"\r
++}\r
++\r
++test_thread_count() {\r
++    test_begin_subtest "${2:-Expecting $1 thread(s)}"\r
++    count=$(notmuch count --output=threads)\r
++    test_expect_equal "$count" "$1"\r
++}\r
++\r
++test_ghost_count() {\r
++    test_begin_subtest "${2:-Expecting $1 ghosts(s)}"\r
++    ghosts=$(../ghost-report ${MAIL_DIR}/.notmuch/xapian)\r
++    test_expect_equal "$ghosts" "$1"\r
++}\r
++\r
++notmuch new >/dev/null\r
++pwd\r
++ls -la ${MAIL_DIR}/.notmuch/xapian\r
++\r
++test_thread_count 0 'There should be no threads initially'\r
++test_ghost_count 0 'There should be no ghosts initially'\r
++\r
++message_a\r
++notmuch new >/dev/null\r
++test_thread_count 1 'One message in: one thread'\r
++test_subject_count apple 1\r
++test_subject_count banana 0\r
++test_ghost_count 0\r
++\r
++message_b\r
++notmuch new >/dev/null\r
++test_thread_count 1 'Second message in the same thread: one thread'\r
++test_subject_count apple 1\r
++test_subject_count banana 1\r
++test_ghost_count 0\r
++\r
++rm -f ${MAIL_DIR}/cur/a\r
++notmuch new >/dev/null\r
++test_thread_count 1 'First message removed: still only one thread'\r
++test_subject_count apple 0\r
++test_subject_count banana 1\r
++test_begin_subtest 'should be one ghost after first message removed'\r
++test_subtest_known_broken\r
++ghosts=$(../ghost-report ${MAIL_DIR}/.notmuch/xapian)\r
++test_expect_equal "$ghosts" "1"\r
++\r
++message_a\r
++notmuch new >/dev/null\r
++# this is known to fail (it shows 2 threads) because no "ghost\r
++# message" was created for message A when it was removed from the\r
++# index, despite message B still pointing to it.\r
++test_begin_subtest 'First message reappears: should return to the same thread'\r
++test_subtest_known_broken\r
++count=$(notmuch count --output=threads)\r
++test_expect_equal "$count" "1"\r
++test_subject_count apple 1\r
++test_subject_count banana 1\r
++test_ghost_count 0\r
++\r
++rm -f ${MAIL_DIR}/cur/b\r
++notmuch new >/dev/null\r
++test_thread_count 1 'Removing second message: still only one thread'\r
++test_subject_count apple 1\r
++test_subject_count banana 0\r
++test_ghost_count 0 'No ghosts should remain after deletion of second message'\r
++\r
++rm -f ${MAIL_DIR}/cur/a\r
++notmuch new >/dev/null\r
++test_thread_count 0 'All messages gone: no threads'\r
++test_subject_count apple 0\r
++test_subject_count banana 0\r
++test_ghost_count 0\r
++\r
++test_done\r
+-- \r
+2.8.0.rc3\r
+\r