[PATCH v3 00/13] Implement and use database "features"
Tomi Ollila
tomi.ollila at iki.fi
Thu Aug 7 14:55:37 PDT 2014
On Fri, Aug 01 2014, Austin Clements <amdragon at MIT.EDU> wrote:
> This is v3 of id:1406652492-27803-1-git-send-email-amdragon at 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 at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list