From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <aclements@csail.mit.edu>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 12/12] lib: Remove unnecessary thread linking steps
	when	using ghost messages
In-Reply-To: <1412637438-4821-13-git-send-email-aclements@csail.mit.edu>
References: <1412637438-4821-1-git-send-email-aclements@csail.mit.edu>
	<1412637438-4821-13-git-send-email-aclements@csail.mit.edu>
User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Wed, 22 Oct 2014 00:17:20 +0100
Message-ID: <87wq7thv27.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Tue, 07 Oct 2014, Austin Clements <aclements@csail.mit.edu> wrote:
> From: Austin Clements <amdragon@mit.edu>
>
> Previously, it was necessary to link new messages to children to work
> around some (though not all) problems with the old metadata-based
> approach to stored thread IDs.  With ghost messages, this is no longer
> necessary, so don't bother with child linking when ghost messages are
> in use.
> ---
>  lib/database.cc | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 1316529..6e51a72 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -2169,10 +2169,23 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,
>      if (status)
>  	goto DONE;
>  
> -    status = _notmuch_database_link_message_to_children (notmuch, message,
> -							 &thread_id);
> -    if (status)
> -	goto DONE;
> +    if (! (notmuch->features & NOTMUCH_FEATURE_GHOSTS)) {
> +	/* In general, it shouldn't be necessary to link children,
> +	 * since the earlier indexing of those children will have
> +	 * stored a thread ID for the missing parent.  However, prior
> +	 * to ghost messages, these stored thread IDs were NOT
> +	 * rewritten during thread merging (and there was no
> +	 * performant way to do so), so if indexed children were
> +	 * pulled into a different thread ID by a merge, it was
> +	 * necessary to pull them *back* into the stored thread ID of
> +	 * the parent.  With ghost messages, we just rewrite the
> +	 * stored thread IDs during merging, so this workaround isn't
> +	 * necessary. */
> +	status = _notmuch_database_link_message_to_children (notmuch, message,
> +							     &thread_id);
> +	if (status)
> +	    goto DONE;
> +    }

Ok so this basically answers my earlier comment. It might be worth
updating the big comment at the start of the function to match the new
code though.

Best wishes

Mark

>  
>      /* If not part of any existing thread, generate a new thread ID. */
>      if (thread_id == NULL) {
> -- 
> 2.1.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch