79/bc58122211fb5988f04499da132bdee20958e7

   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 2C0BD431FAF\r
   6         for <notmuch@notmuchmail.org>; Fri,  8 Aug 2014 11:19:11 -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 BBm2OGBfSuAc for <notmuch@notmuchmail.org>;\r
  16         Fri,  8 Aug 2014 11:19:03 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-2.mit.edu (dmz-mailsec-scanner-2.mit.edu\r
  18         [18.9.25.13])\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 0F853431FAE\r
  22         for <notmuch@notmuchmail.org>; Fri,  8 Aug 2014 11:19:02 -0700 (PDT)\r
  23 X-AuditID: 1209190d-f79c06d000002f07-5e-53e51496d054\r
  24 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\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-2.mit.edu (Symantec Messaging Gateway) with SMTP\r
  28         id BA.A9.12039.69415E35; Fri,  8 Aug 2014 14:19:02 -0400 (EDT)\r
  29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  30         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id s78IJ0xw029781; \r
  31         Fri, 8 Aug 2014 14:19:01 -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 s78IIwn2007149\r
  36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  37         Fri, 8 Aug 2014 14:19:00 -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 1XFokk-0001dF-2r; Fri, 08 Aug 2014 14:18:58 -0400\r
  41 Date: Fri, 8 Aug 2014 14:18:57 -0400\r
  42 From: Austin Clements <amdragon@MIT.EDU>\r
  43 To: Tomi Ollila <tomi.ollila@iki.fi>\r
  44 Subject: Re: [PATCH v3 00/13] Implement and use database "features"\r
  45 Message-ID: <20140808181856.GA17169@mit.edu>\r
  46 References: <1406859003-11561-1-git-send-email-amdragon@mit.edu>\r
  47         <m2a97gt15y.fsf@guru.guru-group.fi>\r
  48 MIME-Version: 1.0\r
  49 Content-Type: text/plain; charset=us-ascii\r
  50 Content-Disposition: inline\r
  51 In-Reply-To: <m2a97gt15y.fsf@guru.guru-group.fi>\r
  52 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  53 X-Brightmail-Tracker:\r
  54  H4sIAAAAAAAAA+NgFmpmleLIzCtJLcpLzFFi42IRYrdT150m8jTYYOUkIYvrN2cyW7xZOY/V\r
  55         gcnj8NeFLB7PVt1iDmCK4rJJSc3JLEst0rdL4Mr4cXUOU8E1hYrfz9+yNTA+kuxi5OSQEDCR\r
  56         2NC8jQnCFpO4cG89G4gtJDCbSeLzitQuRi4gewOjxMo7jUwQzikmif2rDjFDOEsYJeb/2ADW\r
  57         ziKgIvGjZxZYO5uAhsS2/csZQWwRoPiDtvWsIDazgLTEt9/NYPXCAs4Sp442soPYvAI6EjsX\r
  58         9rFCrE6TeLntBjNEXFDi5MwnLBC9WhI3/r0E6uUAm7P8HwdImFPAQGLe8Xlg5aJAq6ac3MY2\r
  59         gVFoFpLuWUi6ZyF0L2BkXsUom5JbpZubmJlTnJqsW5ycmJeXWqRrpJebWaKXmlK6iREU1pyS\r
  60         vDsY3x1UOsQowMGoxMMr0P04WIg1say4MvcQoyQHk5Io7w7+p8FCfEn5KZUZicUZ8UWlOanF\r
  61         hxglOJiVRHifPHoSLMSbklhZlVqUD5OS5mBREud9a20VLCSQnliSmp2aWpBaBJOV4eBQkuC9\r
  62         Jgw0VLAoNT21Ii0zpwQhzcTBCTKcB2j4JZAa3uKCxNzizHSI/ClGRSlxXluQhABIIqM0D64X\r
  63         lnZeMYoDvSLMux2kigeYsuC6XwENZgIaLKv6GGRwSSJCSqqBUUEvLi/BcuqhnLnST7h6sv63\r
  64         LbVLTOBbHHD0idSvXcbS9kctvB9tm2FZa8/qxJEiKLn6c1/UNCOOKfe69W+niydq6N1SWVXf\r
  65         Hcu/MlW7fKWU346yMwciK67OK03N27jP9tYWpfynfGv35BXdijrsvP/14xNR2o9Xa27cx3hY\r
  66         npn9/e2K9VPjlViKMxINtZiLihMBI6E+yRYDAAA=\r
  67 Cc: notmuch@notmuchmail.org\r
  68 X-BeenThere: notmuch@notmuchmail.org\r
  69 X-Mailman-Version: 2.1.13\r
  70 Precedence: list\r
  71 List-Id: "Use and development of the notmuch mail system."\r
  72         <notmuch.notmuchmail.org>\r
  73 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  74         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  75 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  76 List-Post: <mailto:notmuch@notmuchmail.org>\r
  77 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  78 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  79         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  80 X-List-Received-Date: Fri, 08 Aug 2014 18:19:11 -0000\r
  81 \r
  82 Quoth Tomi Ollila on Aug 08 at 12:55 am:\r
  83 > On Fri, Aug 01 2014, Austin Clements <amdragon@MIT.EDU> wrote:\r
  84 > \r
  85 > > This is v3 of id:1406652492-27803-1-git-send-email-amdragon@mit.edu.\r
  86 > > This fixes one issue and tidies up another thing in\r
  87 > > notmuch_database_upgrade I found while working on change tracking\r
  88 > > support.  Most of the patches are logically identical to v2, but the\r
  89 > > changes ripple through the patch context, so I'm sending a new series.\r
  90 > >\r
  91 > > First, this updates notmuch->features before starting the upgrade,\r
  92 > > rather than after, so that functions called by upgrade will use the\r
  93 > > new database features instead of the old (this didn't matter in this\r
  94 > > series because nothing modified the database differently depending on\r
  95 > > features).  Second, this combines multiple _notmuch_message_sync calls\r
  96 > > into one, which cleans up the code, should further improve upgrade\r
  97 > > performance, and makes way for additional per-message upgrades.\r
  98 > \r
  99 > This series looks good to me. I looked through the diffs a few times and\r
 100 > notmuch_database_upgrade() in lib/database.cc to see that in full.\r
 101 > Tests pass (also T530-upgrade now that I downloaded that one test database.)\r
 102 > \r
 103 > I googled answers to few questions along the review; one thing still\r
 104 > interests me -- is there potential to have speed/memory problems\r
 105 > when doing upgrade transaction in large/very large databases. And\r
 106 > how long will the (final) commit_transaction() take (i.e how\r
 107 > many times handle_sigalrm() is called while that is in progress...)\r
 108 > \r
 109 > (my guess is that this transaction just builds a new revision and\r
 110 > if it is never committed the revision is never used -- and data is\r
 111 > written there in some batches of suitable size -- so memory usage\r
 112 > and transaction commit time is O(1))\r
 113 \r
 114 Your guess is basically right.  Xapian periodically flushes stuff to\r
 115 disk during a transaction basically just like it does when not in a\r
 116 transaction; AFAIK the only difference is when it flushes out the new\r
 117 revision number and updated free block metadata.  Hence, speed and\r
 118 memory use aren't affected by the use of a transaction, and\r
 119 commit_transaction isn't particularly sensitive to how big the\r
 120 transaction is.\r
 121 \r
 122 > Tomi\r
 123 > \r
 124 > >\r
 125 > > The diff from v2 is below (excluding patch 2, which David pushed).\r
 126 > >\r
 127 > > diff --git a/lib/database.cc b/lib/database.cc\r
 128 > > index b323691..d90a924 100644\r
 129 > > --- a/lib/database.cc\r
 130 > > +++ b/lib/database.cc\r
 131 > > @@ -1252,6 +1252,10 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 132 > >      /* Perform the upgrade in a transaction. */\r
 133 > >      db->begin_transaction (true);\r
 134 > >  \r
 135 > > +    /* Set the target features so we write out changes in the desired\r
 136 > > +     * format. */\r
 137 > > +    notmuch->features = target_features;\r
 138 > > +\r
 139 > >      /* Perform per-message upgrades. */\r
 140 > >      if (new_features &\r
 141 > >     (NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER)) {\r
 142 > > @@ -1280,7 +1284,6 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 143 > >             if (filename && *filename != '\0') {\r
 144 > >                 _notmuch_message_add_filename (message, filename);\r
 145 > >                 _notmuch_message_clear_data (message);\r
 146 > > -               _notmuch_message_sync (message);\r
 147 > >             }\r
 148 > >             talloc_free (filename);\r
 149 > >         }\r
 150 > > @@ -1289,10 +1292,10 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 151 > >          * probabilistic and stemmed. Change it to the current\r
 152 > >          * boolean prefix. Add "path:" prefixes while at it.\r
 153 > >          */\r
 154 > > -       if (new_features & NOTMUCH_FEATURE_BOOL_FOLDER) {\r
 155 > > +       if (new_features & NOTMUCH_FEATURE_BOOL_FOLDER)\r
 156 > >             _notmuch_message_upgrade_folder (message);\r
 157 > > -           _notmuch_message_sync (message);\r
 158 > > -       }\r
 159 > > +\r
 160 > > +       _notmuch_message_sync (message);\r
 161 > >  \r
 162 > >         notmuch_message_destroy (message);\r
 163 > >  \r
 164 > > @@ -1348,7 +1351,6 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 165 > >     }\r
 166 > >      }\r
 167 > >  \r
 168 > > -    notmuch->features = target_features;\r
 169 > >      db->set_metadata ("features", _print_features (local, notmuch->features));\r
 170 > >      db->set_metadata ("version", STRINGIFY (NOTMUCH_DATABASE_VERSION));\r
 171 > >\r