From f5eb4aa3d915cfa6b277ae5f758d3f61ff6dd9ec Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Sun, 27 Jul 2014 10:35:53 +0100 Subject: [PATCH] Re: [PATCH 08/14] lib: Simplify upgrade code using a transaction --- f5/a8a494c5b10206c5f1f940fd7c9a96e2b1bf55 | 211 ++++++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 f5/a8a494c5b10206c5f1f940fd7c9a96e2b1bf55 diff --git a/f5/a8a494c5b10206c5f1f940fd7c9a96e2b1bf55 b/f5/a8a494c5b10206c5f1f940fd7c9a96e2b1bf55 new file mode 100644 index 000000000..ccc7a0eff --- /dev/null +++ b/f5/a8a494c5b10206c5f1f940fd7c9a96e2b1bf55 @@ -0,0 +1,211 @@ +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 17198431FBC + for ; Sun, 27 Jul 2014 02:36:09 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0.502 +X-Spam-Level: +X-Spam-Status: No, score=0.502 tagged_above=-999 required=5 + tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, + NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_LOW=-0.7] 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 lWWKcPL7ktD6 for ; + Sun, 27 Jul 2014 02:36:01 -0700 (PDT) +Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 740E7431FB6 + for ; Sun, 27 Jul 2014 02:36:01 -0700 (PDT) +Received: from smtp.qmul.ac.uk ([138.37.6.40]) + by mail2.qmul.ac.uk with esmtp (Exim 4.71) + (envelope-from ) + id 1XBKs2-0006Xu-HR; Sun, 27 Jul 2014 10:35:58 +0100 +Received: from 94.196.249.126.threembb.co.uk ([94.196.249.126] helo=localhost) + by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71) + (envelope-from ) + id 1XBKs1-0003Km-Gn; Sun, 27 Jul 2014 10:35:58 +0100 +From: Mark Walters +To: Austin Clements , notmuch@notmuchmail.org +Subject: Re: [PATCH 08/14] lib: Simplify upgrade code using a transaction +In-Reply-To: <1406433173-19169-9-git-send-email-amdragon@mit.edu> +References: <1406433173-19169-1-git-send-email-amdragon@mit.edu> + <1406433173-19169-9-git-send-email-amdragon@mit.edu> +User-Agent: Notmuch/0.15.2+615~g78e3a93 (http://notmuchmail.org) Emacs/23.4.1 + (x86_64-pc-linux-gnu) +Date: Sun, 27 Jul 2014 10:35:53 +0100 +Message-ID: <87vbqjywhy.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Sender-Host-Address: 94.196.249.126 +X-QM-Geographic: According to ripencc, + this message was delivered by a machine in Britain (UK) (GB). +X-QM-SPAM-Info: Sender has good ham record. :) +X-QM-Body-MD5: 429daf3b5226ec8484a887b40bb937aa (of first 20000 bytes) +X-SpamAssassin-Score: -0.1 +X-SpamAssassin-SpamBar: / +X-SpamAssassin-Report: The QM spam filters have analysed this message to + determine if it is + spam. We require at least 5.0 points to mark a message as spam. + This message scored -0.1 points. + Summary of the scoring: + * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail + provider * (markwalters1009[at]gmail.com) + * -0.1 AWL AWL: From: address is in the auto white-list +X-QM-Scan-Virus: ClamAV says the message is clean +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: Sun, 27 Jul 2014 09:36:09 -0000 + + +Hi + +On Sun, 27 Jul 2014, Austin Clements wrote: +> Previously, the upgrade was organized as two passes -- an upgrade +> pass, and a separate cleanup pass -- so the database was always in a +> valid state. This change substantially simplifies this code by +> performing the upgrade in a transaction and combining both passes in +> to one. This 1) eliminates a lot of duplicate code between the +> passes, 2) speeds up the upgrade process, 3) makes progress reporting +> more accurate, 4) eliminates the potential for stale data if the +> upgrade is interrupted during the cleanup pass, and 5) makes it easier +> to reason about the safety of the upgrade code. + +I like this but I wonder if it has a side effect: I think with the +current code the user can interrupt the upgrade (ctrl-c) and continue +roughly where it left off. This looks like it means the whole upgrade +needs to be done in one go. Will this be a problem on large mail stores +(eg rlb with over 1M messages)? + +I am not sure what could be done during the interrupted upgrade before +so maybe this is not a problem. + +Best wishes + +Mark + + +> --- +> lib/database.cc | 67 ++++++--------------------------------------------------- +> 1 file changed, 7 insertions(+), 60 deletions(-) +> +> diff --git a/lib/database.cc b/lib/database.cc +> index 03eef3e..0be7180 100644 +> --- a/lib/database.cc +> +++ b/lib/database.cc +> @@ -1238,6 +1238,9 @@ notmuch_database_upgrade (notmuch_database_t *notmuch, +> timer_is_active = TRUE; +> } +> +> + /* Perform the upgrade in a transaction. */ +> + db->begin_transaction (true); +> + +> /* Before version 1, each message document had its filename in the +> * data field. Copy that into the new format by calling +> * notmuch_message_add_filename. +> @@ -1265,6 +1268,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch, +> filename = _notmuch_message_talloc_copy_data (message); +> if (filename && *filename != '\0') { +> _notmuch_message_add_filename (message, filename); +> + _notmuch_message_clear_data (message); +> _notmuch_message_sync (message); +> } +> talloc_free (filename); +> @@ -1312,6 +1316,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch, +> NOTMUCH_FIND_CREATE, &status); +> notmuch_directory_set_mtime (directory, mtime); +> notmuch_directory_destroy (directory); +> + +> + db->delete_document (*p); +> } +> } +> } +> @@ -1353,67 +1359,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch, +> notmuch->features |= NOTMUCH_FEATURES_CURRENT; +> db->set_metadata ("features", _print_features (local, notmuch->features)); +> db->set_metadata ("version", STRINGIFY (NOTMUCH_DATABASE_VERSION)); +> - db->flush (); +> - +> - /* Now that the upgrade is complete we can remove the old data +> - * and documents that are no longer needed. */ +> - if (version < 1) { +> - notmuch_query_t *query = notmuch_query_create (notmuch, ""); +> - notmuch_messages_t *messages; +> - notmuch_message_t *message; +> - char *filename; +> - +> - for (messages = notmuch_query_search_messages (query); +> - notmuch_messages_valid (messages); +> - notmuch_messages_move_to_next (messages)) +> - { +> - if (do_progress_notify) { +> - progress_notify (closure, (double) count / total); +> - do_progress_notify = 0; +> - } +> - +> - message = notmuch_messages_get (messages); +> - +> - filename = _notmuch_message_talloc_copy_data (message); +> - if (filename && *filename != '\0') { +> - _notmuch_message_clear_data (message); +> - _notmuch_message_sync (message); +> - } +> - talloc_free (filename); +> - +> - notmuch_message_destroy (message); +> - } +> +> - notmuch_query_destroy (query); +> - } +> - +> - if (version < 1) { +> - Xapian::TermIterator t, t_end; +> - +> - t_end = notmuch->xapian_db->allterms_end ("XTIMESTAMP"); +> - +> - for (t = notmuch->xapian_db->allterms_begin ("XTIMESTAMP"); +> - t != t_end; +> - t++) +> - { +> - Xapian::PostingIterator p, p_end; +> - std::string term = *t; +> - +> - p_end = notmuch->xapian_db->postlist_end (term); +> - +> - for (p = notmuch->xapian_db->postlist_begin (term); +> - p != p_end; +> - p++) +> - { +> - if (do_progress_notify) { +> - progress_notify (closure, (double) count / total); +> - do_progress_notify = 0; +> - } +> - +> - db->delete_document (*p); +> - } +> - } +> - } +> + db->commit_transaction (); +> +> if (timer_is_active) { +> /* Now stop the timer. */ +> -- +> 2.0.0 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch -- 2.26.2