From 61648ff17b2ee8795025f6f259a51ccd2b62c776 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sat, 9 Aug 2014 14:18:57 +2000 Subject: [PATCH] Re: [PATCH v3 00/13] Implement and use database "features" --- 79/bc58122211fb5988f04499da132bdee20958e7 | 171 ++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 79/bc58122211fb5988f04499da132bdee20958e7 diff --git a/79/bc58122211fb5988f04499da132bdee20958e7 b/79/bc58122211fb5988f04499da132bdee20958e7 new file mode 100644 index 000000000..438ea40b4 --- /dev/null +++ b/79/bc58122211fb5988f04499da132bdee20958e7 @@ -0,0 +1,171 @@ +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 2C0BD431FAF + for ; Fri, 8 Aug 2014 11:19:11 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -2.3 +X-Spam-Level: +X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_MED=-2.3] 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 BBm2OGBfSuAc for ; + Fri, 8 Aug 2014 11:19:03 -0700 (PDT) +Received: from dmz-mailsec-scanner-2.mit.edu (dmz-mailsec-scanner-2.mit.edu + [18.9.25.13]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 0F853431FAE + for ; Fri, 8 Aug 2014 11:19:02 -0700 (PDT) +X-AuditID: 1209190d-f79c06d000002f07-5e-53e51496d054 +Received: from mailhub-auth-4.mit.edu ( [18.7.62.39]) + (using TLS with cipher AES256-SHA (256/256 bits)) + (Client did not present a certificate) + by dmz-mailsec-scanner-2.mit.edu (Symantec Messaging Gateway) with SMTP + id BA.A9.12039.69415E35; Fri, 8 Aug 2014 14:19:02 -0400 (EDT) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id s78IJ0xw029781; + Fri, 8 Aug 2014 14:19:01 -0400 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id s78IIwn2007149 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Fri, 8 Aug 2014 14:19:00 -0400 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1XFokk-0001dF-2r; Fri, 08 Aug 2014 14:18:58 -0400 +Date: Fri, 8 Aug 2014 14:18:57 -0400 +From: Austin Clements +To: Tomi Ollila +Subject: Re: [PATCH v3 00/13] Implement and use database "features" +Message-ID: <20140808181856.GA17169@mit.edu> +References: <1406859003-11561-1-git-send-email-amdragon@mit.edu> + +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFmpmleLIzCtJLcpLzFFi42IRYrdT150m8jTYYOUkIYvrN2cyW7xZOY/V + gcnj8NeFLB7PVt1iDmCK4rJJSc3JLEst0rdL4Mr4cXUOU8E1hYrfz9+yNTA+kuxi5OSQEDCR + 2NC8jQnCFpO4cG89G4gtJDCbSeLzitQuRi4gewOjxMo7jUwQzikmif2rDjFDOEsYJeb/2ADW + ziKgIvGjZxZYO5uAhsS2/csZQWwRoPiDtvWsIDazgLTEt9/NYPXCAs4Sp442soPYvAI6EjsX + 9rFCrE6TeLntBjNEXFDi5MwnLBC9WhI3/r0E6uUAm7P8HwdImFPAQGLe8Xlg5aJAq6ac3MY2 + gVFoFpLuWUi6ZyF0L2BkXsUom5JbpZubmJlTnJqsW5ycmJeXWqRrpJebWaKXmlK6iREU1pyS + vDsY3x1UOsQowMGoxMMr0P04WIg1say4MvcQoyQHk5Io7w7+p8FCfEn5KZUZicUZ8UWlOanF + hxglOJiVRHifPHoSLMSbklhZlVqUD5OS5mBREud9a20VLCSQnliSmp2aWpBaBJOV4eBQkuC9 + Jgw0VLAoNT21Ii0zpwQhzcTBCTKcB2j4JZAa3uKCxNzizHSI/ClGRSlxXluQhABIIqM0D64X + lnZeMYoDvSLMux2kigeYsuC6XwENZgIaLKv6GGRwSSJCSqqBUUEvLi/BcuqhnLnST7h6sv63 + LbVLTOBbHHD0idSvXcbS9kctvB9tm2FZa8/qxJEiKLn6c1/UNCOOKfe69W+niydq6N1SWVXf + Hcu/MlW7fKWU346yMwciK67OK03N27jP9tYWpfynfGv35BXdijrsvP/14xNR2o9Xa27cx3hY + npn9/e2K9VPjlViKMxINtZiLihMBI6E+yRYDAAA= +Cc: notmuch@notmuchmail.org +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: Fri, 08 Aug 2014 18:19:11 -0000 + +Quoth Tomi Ollila on Aug 08 at 12:55 am: +> 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)) + +Your guess is basically right. Xapian periodically flushes stuff to +disk during a transaction basically just like it does when not in a +transaction; AFAIK the only difference is when it flushes out the new +revision number and updated free block metadata. Hence, speed and +memory use aren't affected by the use of a transaction, and +commit_transaction isn't particularly sensitive to how big the +transaction is. + +> 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)); +> > -- 2.26.2