Re: [PATCH v3 00/13] Implement and use database "features"

diff --git a/19/8df12fc664f7f26c0713a3ac42578f67874f53 b/19/8df12fc664f7f26c0713a3ac42578f67874f53
new file mode 100644 (file)
index 0000000..ade8c0a
--- /dev/null
+++ b/19/8df12fc664f7f26c0713a3ac42578f67874f53
@@ -0,0 +1,135 @@
+Return-Path: <tomi.ollila@iki.fi>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id 3221E431FAF\r
+       for <notmuch@notmuchmail.org>; Thu,  7 Aug 2014 14:55:58 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.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 olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id UlEBHfOD1Xny for <notmuch@notmuchmail.org>;\r
+       Thu,  7 Aug 2014 14:55:50 -0700 (PDT)\r
+Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
+       by olra.theworths.org (Postfix) with ESMTP id 9A728431FAE\r
+       for <notmuch@notmuchmail.org>; Thu,  7 Aug 2014 14:55:49 -0700 (PDT)\r
+Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
+       by guru.guru-group.fi (Postfix) with ESMTP id 8D160100051;\r
+       Fri,  8 Aug 2014 00:55:37 +0300 (EEST)\r
+From: Tomi Ollila <tomi.ollila@iki.fi>\r
+To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v3 00/13] Implement and use database "features"\r
+In-Reply-To: <1406859003-11561-1-git-send-email-amdragon@mit.edu>\r
+References: <1406859003-11561-1-git-send-email-amdragon@mit.edu>\r
+User-Agent: Notmuch/0.18.1+25~gdaf4b6f (http://notmuchmail.org) Emacs/24.3.1\r
+       (x86_64-unknown-linux-gnu)\r
+X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
+       $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
+       !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
+Date: Fri, 08 Aug 2014 00:55:37 +0300\r
+Message-ID: <m2a97gt15y.fsf@guru.guru-group.fi>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://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: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Thu, 07 Aug 2014 21:55:58 -0000\r
+\r
+On Fri, Aug 01 2014, Austin Clements <amdragon@MIT.EDU> wrote:\r
+\r
+> This is v3 of id:1406652492-27803-1-git-send-email-amdragon@mit.edu.\r
+> This fixes one issue and tidies up another thing in\r
+> notmuch_database_upgrade I found while working on change tracking\r
+> support.  Most of the patches are logically identical to v2, but the\r
+> changes ripple through the patch context, so I'm sending a new series.\r
+>\r
+> First, this updates notmuch->features before starting the upgrade,\r
+> rather than after, so that functions called by upgrade will use the\r
+> new database features instead of the old (this didn't matter in this\r
+> series because nothing modified the database differently depending on\r
+> features).  Second, this combines multiple _notmuch_message_sync calls\r
+> into one, which cleans up the code, should further improve upgrade\r
+> performance, and makes way for additional per-message upgrades.\r
+\r
+This series looks good to me. I looked through the diffs a few times and\r
+notmuch_database_upgrade() in lib/database.cc to see that in full.\r
+Tests pass (also T530-upgrade now that I downloaded that one test database.)\r
+\r
+I googled answers to few questions along the review; one thing still\r
+interests me -- is there potential to have speed/memory problems\r
+when doing upgrade transaction in large/very large databases. And\r
+how long will the (final) commit_transaction() take (i.e how\r
+many times handle_sigalrm() is called while that is in progress...)\r
+\r
+(my guess is that this transaction just builds a new revision and\r
+if it is never committed the revision is never used -- and data is\r
+written there in some batches of suitable size -- so memory usage\r
+and transaction commit time is O(1))\r
+\r
+Tomi\r
+\r
+>\r
+> The diff from v2 is below (excluding patch 2, which David pushed).\r
+>\r
+> diff --git a/lib/database.cc b/lib/database.cc\r
+> index b323691..d90a924 100644\r
+> --- a/lib/database.cc\r
+> +++ b/lib/database.cc\r
+> @@ -1252,6 +1252,10 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
+>      /* Perform the upgrade in a transaction. */\r
+>      db->begin_transaction (true);\r
+>  \r
+> +    /* Set the target features so we write out changes in the desired\r
+> +     * format. */\r
+> +    notmuch->features = target_features;\r
+> +\r
+>      /* Perform per-message upgrades. */\r
+>      if (new_features &\r
+>      (NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER)) {\r
+> @@ -1280,7 +1284,6 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
+>              if (filename && *filename != '\0') {\r
+>                  _notmuch_message_add_filename (message, filename);\r
+>                  _notmuch_message_clear_data (message);\r
+> -                _notmuch_message_sync (message);\r
+>              }\r
+>              talloc_free (filename);\r
+>          }\r
+> @@ -1289,10 +1292,10 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
+>           * probabilistic and stemmed. Change it to the current\r
+>           * boolean prefix. Add "path:" prefixes while at it.\r
+>           */\r
+> -        if (new_features & NOTMUCH_FEATURE_BOOL_FOLDER) {\r
+> +        if (new_features & NOTMUCH_FEATURE_BOOL_FOLDER)\r
+>              _notmuch_message_upgrade_folder (message);\r
+> -            _notmuch_message_sync (message);\r
+> -        }\r
+> +\r
+> +        _notmuch_message_sync (message);\r
+>  \r
+>          notmuch_message_destroy (message);\r
+>  \r
+> @@ -1348,7 +1351,6 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
+>      }\r
+>      }\r
+>  \r
+> -    notmuch->features = target_features;\r
+>      db->set_metadata ("features", _print_features (local, notmuch->features));\r
+>      db->set_metadata ("version", STRINGIFY (NOTMUCH_DATABASE_VERSION));\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r