RE: [PATCH 4/5] Introduce g:notmuch_rb_folders_count_threads
[notmuch-archives.git] / a4 / 9c51782b4aaab3cc36e03600e1354409cb06e0
1 Return-Path: <amdragon@gmail.com>\r
2 X-Original-To: notmuch@notmuchmail.org\r
3 Delivered-To: notmuch@notmuchmail.org\r
4 Received: from localhost (localhost [127.0.0.1])\r
5         by olra.theworths.org (Postfix) with ESMTP id 89B70429E5F\r
6         for <notmuch@notmuchmail.org>; Tue, 28 Jun 2011 23:37:02 -0700 (PDT)\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
8 X-Spam-Flag: NO\r
9 X-Spam-Score: -0.699\r
10 X-Spam-Level: \r
11 X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5\r
12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001,\r
13         RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
14 Received: from olra.theworths.org ([127.0.0.1])\r
15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
16         with ESMTP id PPHPPIMheAYw for <notmuch@notmuchmail.org>;\r
17         Tue, 28 Jun 2011 23:37:01 -0700 (PDT)\r
18 Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com\r
19         [209.85.216.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
20         (No client certificate requested)\r
21         by olra.theworths.org (Postfix) with ESMTPS id BAED1429E55\r
22         for <notmuch@notmuchmail.org>; Tue, 28 Jun 2011 23:37:01 -0700 (PDT)\r
23 Received: by qyk9 with SMTP id 9so602678qyk.5\r
24         for <notmuch@notmuchmail.org>; Tue, 28 Jun 2011 23:37:01 -0700 (PDT)\r
25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;\r
26         h=mime-version:sender:in-reply-to:references:date\r
27         :x-google-sender-auth:message-id:subject:from:to:cc:content-type;\r
28         bh=/FU4WYtOFn5Z8Cv3EzJR1Kb2vY0jR2vt26zV/cEEZ2s=;\r
29         b=C8mSQBKBW7owiEA8lGHe5xncUo9jJ4/8YvM8bkAwg4unbwI3lUAD0S82V1DHcs1/Vi\r
30         2aFQGG7Sf+YeM22ePu/z8R8qEt08oaZ8UUD9A5sCcXhwx+3UURpNl5rLfhijFRTnifmJ\r
31         YC+wFF7whot+IryT2V9it8N+8oa0N2LWv1Vuk=\r
32 MIME-Version: 1.0\r
33 Received: by 10.229.106.32 with SMTP id v32mr299999qco.77.1309329420907; Tue,\r
34         28 Jun 2011 23:37:00 -0700 (PDT)\r
35 Sender: amdragon@gmail.com\r
36 Received: by 10.229.249.193 with HTTP; Tue, 28 Jun 2011 23:37:00 -0700 (PDT)\r
37 In-Reply-To: <BANLkTimeWnn5YtKjarFB8f8Fowvz0PxK2Q@mail.gmail.com>\r
38 References: <87ei34rnc5.fsf@yoom.home.cworth.org>\r
39         <1307822683-848-1-git-send-email-amdragon@mit.edu>\r
40         <BANLkTimeWnn5YtKjarFB8f8Fowvz0PxK2Q@mail.gmail.com>\r
41 Date: Wed, 29 Jun 2011 02:37:00 -0400\r
42 X-Google-Sender-Auth: Lk8gItFyysmvF-s4Eqe4-U1WkQM\r
43 Message-ID:\r
44  <CAH-f9Wt9ZH0rMsaCK1bMfsccTGhYvcBBAAH=dSL5ps+KhyHtwA@mail.gmail.com>\r
45 Subject: Re: [PATCH v6 00/17] Fix 'notmuch new' atomicity issues\r
46 From: Austin Clements <amdragon@mit.edu>\r
47 To: Carl Worth <cworth@cworth.org>\r
48 Content-Type: text/plain; charset=ISO-8859-1\r
49 Cc: notmuch@notmuchmail.org\r
50 X-BeenThere: notmuch@notmuchmail.org\r
51 X-Mailman-Version: 2.1.13\r
52 Precedence: list\r
53 List-Id: "Use and development of the notmuch mail system."\r
54         <notmuch.notmuchmail.org>\r
55 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
56         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
57 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
58 List-Post: <mailto:notmuch@notmuchmail.org>\r
59 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
60 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
61         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
62 X-List-Received-Date: Wed, 29 Jun 2011 06:37:02 -0000\r
63 \r
64 Just found one more atomicity bug in notmuch-new.  I would prefer to\r
65 not update this patch series yet again, but rather to follow up with a\r
66 separate fix for this.  The full bug is described below, but the gist\r
67 is that how add_files_recursive computes new_directory from the mtime\r
68 isn't sound.  The simplest fix is to remove the new_directory\r
69 optimization, but then we wouldn't scan files in inode order.  The\r
70 best fix is probably to add an out-argument to\r
71 notmuch_database_get_directory that indicates if the directory really\r
72 was just created in the database (and hence has no files or subdirs).\r
73 Unfortunately, that requires an API change.  If that's undesirable, an\r
74 alternate would be to record that bit in the notmuch_directory_t and\r
75 add some notmuch_directory_is_new API that returns it.  Thoughts?\r
76 \r
77 \r
78 Bug description:\r
79 \r
80 add_files_recursive determines whether a directory is "new" by\r
81 inspecting the database mtime returned by notmuch_directory_get_mtime.\r
82  However, if there's an interruption after adding\r
83 messages/subdirectories from a new directory but before updating the\r
84 directory's mtime, it will be considered a new directory again on the\r
85 next run.  As a result, on the next run, db_files and db_subdirs will\r
86 not be loaded from the database (even though they *wouldn't* be NULL).\r
87  As a result, we'll miss removing messages or entire subdirectories\r
88 that have been deleted from the "new" directory.  (We'll also re-add\r
89 the messages in this directory to the database rather than skipping\r
90 them, which will throw off the notmuch new statistics, but that's\r
91 fine.)\r
92 \r
93 This didn't show up in the atomicity test because throwing off\r
94 anything besides the statistics requires *two* interruptions.  In\r
95 fact, I don't even know how I would write an automated test for this.\r