fc/ace1ee0e3e0dad7b16af2d89225739a877b94c

   1 Return-Path: <amdragon@mit.edu>\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 14E25431FB6\r
   6         for <notmuch@notmuchmail.org>; Sun, 27 Jul 2014 09:42:37 -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: -2.3\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id ImwWbGeucHTM for <notmuch@notmuchmail.org>;\r
  16         Sun, 27 Jul 2014 09:42:29 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-7.mit.edu (dmz-mailsec-scanner-7.mit.edu\r
  18         [18.7.68.36])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 2E156431FAE\r
  22         for <notmuch@notmuchmail.org>; Sun, 27 Jul 2014 09:42:29 -0700 (PDT)\r
  23 X-AuditID: 12074424-f79146d00000067c-d8-53d52bf43424\r
  24 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  25         (using TLS with cipher AES256-SHA (256/256 bits))\r
  26         (Client did not present a certificate)\r
  27         by dmz-mailsec-scanner-7.mit.edu (Symantec Messaging Gateway) with SMTP\r
  28         id 4F.A3.01660.4FB25D35; Sun, 27 Jul 2014 12:42:28 -0400 (EDT)\r
  29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  30         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id s6RGgRmg012752; \r
  31         Sun, 27 Jul 2014 12:42:28 -0400\r
  32 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  33         (authenticated bits=0)\r
  34         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  35         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id s6RGgPqo019190\r
  36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  37         Sun, 27 Jul 2014 12:42:26 -0400\r
  38 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  39         (envelope-from <amdragon@mit.edu>)\r
  40         id 1XBRWi-00079H-Ri; Sun, 27 Jul 2014 12:42:24 -0400\r
  41 Date: Sun, 27 Jul 2014 12:42:24 -0400\r
  42 From: Austin Clements <amdragon@MIT.EDU>\r
  43 To: Mark Walters <markwalters1009@gmail.com>\r
  44 Subject: Re: [PATCH 08/14] lib: Simplify upgrade code using a transaction\r
  45 Message-ID: <20140727164224.GG13893@mit.edu>\r
  46 References: <1406433173-19169-1-git-send-email-amdragon@mit.edu>\r
  47         <1406433173-19169-9-git-send-email-amdragon@mit.edu>\r
  48         <87vbqjywhy.fsf@qmul.ac.uk>\r
  49 MIME-Version: 1.0\r
  50 Content-Type: text/plain; charset=us-ascii\r
  51 Content-Disposition: inline\r
  52 In-Reply-To: <87vbqjywhy.fsf@qmul.ac.uk>\r
  53 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  54 X-Brightmail-Tracker:\r
  55  H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IR4hTV1v2ifTXYoG++ssXquTwW12/OZHZg\r
  56         8tg56y67x7NVt5gDmKK4bFJSczLLUov07RK4Mp69yS44olsxee9nlgbG50pdjJwcEgImElfu\r
  57         bmCHsMUkLtxbz9bFyMUhJDCbSeLjwS9QzkZGifkt+5kgnNNMEpN61rJAOEsYJZZeXckC0s8i\r
  58         oCpxd+1kNhCbTUBDYtv+5YwgtoiAjsTtQwvAdjALSEt8+90MNImDQ1jAU+LLoUKQMC9QSc+J\r
  59         71AzpzJKvH7TxwyREJQ4OfMJC0SvlsSNfy/BekHmLP/HARLmBFp1fO8vsFWiAioSU05uY5vA\r
  60         KDQLSfcsJN2zELoXMDKvYpRNya3SzU3MzClOTdYtTk7My0st0jXXy80s0UtNKd3ECApqdheV\r
  61         HYzNh5QOMQpwMCrx8GawXQkWYk0sK67MPcQoycGkJMqrDYwJIb6k/JTKjMTijPii0pzU4kOM\r
  62         EhzMSiK8hW+BynlTEiurUovyYVLSHCxK4rxvra2ChQTSE0tSs1NTC1KLYLIyHBxKErx/tICG\r
  63         ChalpqdWpGXmlCCkmTg4QYbzAA2frwFUw1tckJhbnJkOkT/FqCglznsSpFkAJJFRmgfXC0s6\r
  64         rxjFgV4R5uUAuZsHmLDgul8BDWYCGszifxlkcEkiQkqqgXGO3e1Aee3sjCVHL8xWjPrUqWrh\r
  65         /kHf2fb0U8dTplN2C1sW6rodU1Mt/JbQpcz8/2Pqj/5zEWZZigai2/a3anrO2FJcuSjRhkU+\r
  66         PONP5sx7J8MjarbufxXkfHSiarDul5QqV5bSz3d19hbeczzKxraq61rC4q4tUfnZPBe3L2ec\r
  67         faz3TdDjeCWW4oxEQy3mouJEAEbJL1YVAwAA\r
  68 Cc: notmuch@notmuchmail.org\r
  69 X-BeenThere: notmuch@notmuchmail.org\r
  70 X-Mailman-Version: 2.1.13\r
  71 Precedence: list\r
  72 List-Id: "Use and development of the notmuch mail system."\r
  73         <notmuch.notmuchmail.org>\r
  74 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  76 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  77 List-Post: <mailto:notmuch@notmuchmail.org>\r
  78 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  79 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  80         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  81 X-List-Received-Date: Sun, 27 Jul 2014 16:42:37 -0000\r
  82 \r
  83 Quoth Mark Walters on Jul 27 at 10:35 am:\r
  84 > \r
  85 > Hi\r
  86 > \r
  87 > On Sun, 27 Jul 2014, Austin Clements <amdragon@MIT.EDU> wrote:\r
  88 > > Previously, the upgrade was organized as two passes -- an upgrade\r
  89 > > pass, and a separate cleanup pass -- so the database was always in a\r
  90 > > valid state.  This change substantially simplifies this code by\r
  91 > > performing the upgrade in a transaction and combining both passes in\r
  92 > > to one.  This 1) eliminates a lot of duplicate code between the\r
  93 > > passes, 2) speeds up the upgrade process, 3) makes progress reporting\r
  94 > > more accurate, 4) eliminates the potential for stale data if the\r
  95 > > upgrade is interrupted during the cleanup pass, and 5) makes it easier\r
  96 > > to reason about the safety of the upgrade code.\r
  97 > \r
  98 > I like this but I wonder if it has a side effect: I think with the\r
  99 > current code the user can interrupt the upgrade (ctrl-c) and continue\r
 100 > roughly where it left off. This looks like it means the whole upgrade\r
 101 > needs to be done in one go. Will this be a problem on large mail stores\r
 102 > (eg rlb with over 1M messages)?\r
 103 > \r
 104 > I am not sure what could be done during the interrupted upgrade before\r
 105 > so maybe this is not a problem.\r
 106 \r
 107 I haven't tested this hypothesis, but I don't think a partially\r
 108 completed upgrade would actually help upon restarting the upgrade.\r
 109 Since the old upgrade process couldn't safely remove terms/data until\r
 110 the end of the upgrade, if it were interrupted, the next upgrade would\r
 111 start right back at the beginning and do everything over again.\r
 112 \r
 113 Also, since the old upgrade code had to update the version number\r
 114 before removing old terms/data, if it was interrupted during the\r
 115 cleanup process the database would be left with cruft that would\r
 116 *never* be removed.\r
 117 \r
 118 With features we actually have a better chance of making partially\r
 119 completed upgrades useful: we could commit after each individual\r
 120 feature gets upgraded.  Of course, that only helps when upgrade has\r
 121 multiple new features to upgrade to, so it may or may not be useful in\r
 122 practice depending on how quickly we add new features.\r
 123 \r
 124 > Best wishes\r
 125 > \r
 126 > Mark\r
 127 > \r
 128 > \r
 129 > > ---\r
 130 > >  lib/database.cc | 67 ++++++---------------------------------------------------\r
 131 > >  1 file changed, 7 insertions(+), 60 deletions(-)\r
 132 > >\r
 133 > > diff --git a/lib/database.cc b/lib/database.cc\r
 134 > > index 03eef3e..0be7180 100644\r
 135 > > --- a/lib/database.cc\r
 136 > > +++ b/lib/database.cc\r
 137 > > @@ -1238,6 +1238,9 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 138 > >     timer_is_active = TRUE;\r
 139 > >      }\r
 140 > >  \r
 141 > > +    /* Perform the upgrade in a transaction. */\r
 142 > > +    db->begin_transaction (true);\r
 143 > > +\r
 144 > >      /* Before version 1, each message document had its filename in the\r
 145 > >       * data field. Copy that into the new format by calling\r
 146 > >       * notmuch_message_add_filename.\r
 147 > > @@ -1265,6 +1268,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 148 > >         filename = _notmuch_message_talloc_copy_data (message);\r
 149 > >         if (filename && *filename != '\0') {\r
 150 > >             _notmuch_message_add_filename (message, filename);\r
 151 > > +           _notmuch_message_clear_data (message);\r
 152 > >             _notmuch_message_sync (message);\r
 153 > >         }\r
 154 > >         talloc_free (filename);\r
 155 > > @@ -1312,6 +1316,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 156 > >                                                    NOTMUCH_FIND_CREATE, &status);\r
 157 > >             notmuch_directory_set_mtime (directory, mtime);\r
 158 > >             notmuch_directory_destroy (directory);\r
 159 > > +\r
 160 > > +           db->delete_document (*p);\r
 161 > >         }\r
 162 > >     }\r
 163 > >      }\r
 164 > > @@ -1353,67 +1359,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 165 > >      notmuch->features |= NOTMUCH_FEATURES_CURRENT;\r
 166 > >      db->set_metadata ("features", _print_features (local, notmuch->features));\r
 167 > >      db->set_metadata ("version", STRINGIFY (NOTMUCH_DATABASE_VERSION));\r
 168 > > -    db->flush ();\r
 169 > > -\r
 170 > > -    /* Now that the upgrade is complete we can remove the old data\r
 171 > > -     * and documents that are no longer needed. */\r
 172 > > -    if (version < 1) {\r
 173 > > -   notmuch_query_t *query = notmuch_query_create (notmuch, "");\r
 174 > > -   notmuch_messages_t *messages;\r
 175 > > -   notmuch_message_t *message;\r
 176 > > -   char *filename;\r
 177 > > -\r
 178 > > -   for (messages = notmuch_query_search_messages (query);\r
 179 > > -        notmuch_messages_valid (messages);\r
 180 > > -        notmuch_messages_move_to_next (messages))\r
 181 > > -   {\r
 182 > > -       if (do_progress_notify) {\r
 183 > > -           progress_notify (closure, (double) count / total);\r
 184 > > -           do_progress_notify = 0;\r
 185 > > -       }\r
 186 > > -\r
 187 > > -       message = notmuch_messages_get (messages);\r
 188 > > -\r
 189 > > -       filename = _notmuch_message_talloc_copy_data (message);\r
 190 > > -       if (filename && *filename != '\0') {\r
 191 > > -           _notmuch_message_clear_data (message);\r
 192 > > -           _notmuch_message_sync (message);\r
 193 > > -       }\r
 194 > > -       talloc_free (filename);\r
 195 > > -\r
 196 > > -       notmuch_message_destroy (message);\r
 197 > > -   }\r
 198 > >  \r
 199 > > -   notmuch_query_destroy (query);\r
 200 > > -    }\r
 201 > > -\r
 202 > > -    if (version < 1) {\r
 203 > > -   Xapian::TermIterator t, t_end;\r
 204 > > -\r
 205 > > -   t_end = notmuch->xapian_db->allterms_end ("XTIMESTAMP");\r
 206 > > -\r
 207 > > -   for (t = notmuch->xapian_db->allterms_begin ("XTIMESTAMP");\r
 208 > > -        t != t_end;\r
 209 > > -        t++)\r
 210 > > -   {\r
 211 > > -       Xapian::PostingIterator p, p_end;\r
 212 > > -       std::string term = *t;\r
 213 > > -\r
 214 > > -       p_end = notmuch->xapian_db->postlist_end (term);\r
 215 > > -\r
 216 > > -       for (p = notmuch->xapian_db->postlist_begin (term);\r
 217 > > -            p != p_end;\r
 218 > > -            p++)\r
 219 > > -       {\r
 220 > > -           if (do_progress_notify) {\r
 221 > > -               progress_notify (closure, (double) count / total);\r
 222 > > -               do_progress_notify = 0;\r
 223 > > -           }\r
 224 > > -\r
 225 > > -           db->delete_document (*p);\r
 226 > > -       }\r
 227 > > -   }\r
 228 > > -    }\r
 229 > > +    db->commit_transaction ();\r
 230 > >  \r
 231 > >      if (timer_is_active) {\r
 232 > >     /* Now stop the timer. */\r
 233 > >\r