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