[RFC patch 2/2] lib: handle DatabaseModifiedError in _n_message_ensure_metadata

Subject: [RFC patch 2/2] lib: handle DatabaseModifiedError in _n_message_ensure_metadata

Date: Thu, 23 Feb 2017 22:00:48 -0400

To: David Bremner, Gaute Hope, notmuch@notmuchmail.org

Cc:

From: David Bremner


The error handling here still needs work. The retry count should be
handled in more sane way, and both running out of retries and an error
return from notmuch_database_reopen should be handled.
---
 lib/message.cc | 144 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 80 insertions(+), 64 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 2fb67d85..b7c377fc 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -310,10 +310,12 @@ _notmuch_message_get_term (notmuch_message_t *message,
     return value;
 }
 
-static void
+void
 _notmuch_message_ensure_metadata (notmuch_message_t *message)
 {
     Xapian::TermIterator i, end;
+    notmuch_bool_t success = FALSE;
+
     const char *thread_prefix = _find_prefix ("thread"),
 	*tag_prefix = _find_prefix ("tag"),
 	*id_prefix = _find_prefix ("id"),
@@ -327,73 +329,87 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message)
      * slightly more costly than looking up individual fields if only
      * one field of the message object is actually used, it's a huge
      * win as more fields are used. */
+    for (int count=0; count < 100 && !success; count++) {
+	try {
+	    i = message->doc.termlist_begin ();
+	    end = message->doc.termlist_end ();
+
+	    /* Get thread */
+	    if (!message->thread_id)
+		message->thread_id =
+		    _notmuch_message_get_term (message, i, end, thread_prefix);
+
+	    /* Get tags */
+	    assert (strcmp (thread_prefix, tag_prefix) < 0);
+	    if (!message->tag_list) {
+		message->tag_list =
+		    _notmuch_database_get_terms_with_prefix (message, i, end,
+							     tag_prefix);
+		_notmuch_string_list_sort (message->tag_list);
+	    }
 
-    i = message->doc.termlist_begin ();
-    end = message->doc.termlist_end ();
-
-    /* Get thread */
-    if (!message->thread_id)
-	message->thread_id =
-	    _notmuch_message_get_term (message, i, end, thread_prefix);
-
-    /* Get tags */
-    assert (strcmp (thread_prefix, tag_prefix) < 0);
-    if (!message->tag_list) {
-	message->tag_list =
-	    _notmuch_database_get_terms_with_prefix (message, i, end,
-						     tag_prefix);
-	_notmuch_string_list_sort (message->tag_list);
-    }
+	    /* Get id */
+	    assert (strcmp (tag_prefix, id_prefix) < 0);
+	    if (!message->message_id)
+		message->message_id =
+		    _notmuch_message_get_term (message, i, end, id_prefix);
+
+	    /* Get document type */
+	    assert (strcmp (id_prefix, type_prefix) < 0);
+	    if (! NOTMUCH_TEST_BIT (message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST)) {
+		i.skip_to (type_prefix);
+		/* "T" is the prefix "type" fields.  See
+		 * BOOLEAN_PREFIX_INTERNAL. */
+		if (*i == "Tmail")
+		    NOTMUCH_CLEAR_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
+		else if (*i == "Tghost")
+		    NOTMUCH_SET_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
+		else
+		    INTERNAL_ERROR ("Message without type term");
+		NOTMUCH_SET_BIT (&message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST);
+	    }
 
-    /* Get id */
-    assert (strcmp (tag_prefix, id_prefix) < 0);
-    if (!message->message_id)
-	message->message_id =
-	    _notmuch_message_get_term (message, i, end, id_prefix);
-
-    /* Get document type */
-    assert (strcmp (id_prefix, type_prefix) < 0);
-    if (! NOTMUCH_TEST_BIT (message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST)) {
-	i.skip_to (type_prefix);
-	/* "T" is the prefix "type" fields.  See
-	 * BOOLEAN_PREFIX_INTERNAL. */
-	if (*i == "Tmail")
-	    NOTMUCH_CLEAR_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
-	else if (*i == "Tghost")
-	    NOTMUCH_SET_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
-	else
-	    INTERNAL_ERROR ("Message without type term");
-	NOTMUCH_SET_BIT (&message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST);
+	    /* Get filename list.  Here we get only the terms.  We lazily
+	     * expand them to full file names when needed in
+	     * _notmuch_message_ensure_filename_list. */
+	    assert (strcmp (type_prefix, filename_prefix) < 0);
+	    if (!message->filename_term_list && !message->filename_list)
+		message->filename_term_list =
+		    _notmuch_database_get_terms_with_prefix (message, i, end,
+							     filename_prefix);
+
+
+	    /* Get property terms. Mimic the setup with filenames above */
+	    assert (strcmp (filename_prefix, property_prefix) < 0);
+	    if (!message->property_map && !message->property_term_list)
+		message->property_term_list =
+		    _notmuch_database_get_terms_with_prefix (message, i, end,
+							 property_prefix);
+
+	    /* Get reply to */
+	    assert (strcmp (property_prefix, replyto_prefix) < 0);
+	    if (!message->in_reply_to)
+		message->in_reply_to =
+		    _notmuch_message_get_term (message, i, end, replyto_prefix);
+
+
+	    /* It's perfectly valid for a message to have no In-Reply-To
+	     * header. For these cases, we return an empty string. */
+	    if (!message->in_reply_to)
+		message->in_reply_to = talloc_strdup (message, "");
+
+	    /* all the way without an exception */
+	    success = TRUE;
+	} catch (const Xapian::DatabaseModifiedError &error) {
+	    (void) notmuch_database_reopen (message->notmuch);
+	    success = FALSE;
+	} catch (const Xapian::Error &error) {
+	    _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred fetching message metadata\n",
+				  error.get_msg().c_str());
+	    message->notmuch->exception_reported = TRUE;
+	}
     }
 
-    /* Get filename list.  Here we get only the terms.  We lazily
-     * expand them to full file names when needed in
-     * _notmuch_message_ensure_filename_list. */
-    assert (strcmp (type_prefix, filename_prefix) < 0);
-    if (!message->filename_term_list && !message->filename_list)
-	message->filename_term_list =
-	    _notmuch_database_get_terms_with_prefix (message, i, end,
-						     filename_prefix);
-
-
-    /* Get property terms. Mimic the setup with filenames above */
-    assert (strcmp (filename_prefix, property_prefix) < 0);
-    if (!message->property_map && !message->property_term_list)
-	message->property_term_list =
-	    _notmuch_database_get_terms_with_prefix (message, i, end,
-						     property_prefix);
-
-    /* Get reply to */
-    assert (strcmp (property_prefix, replyto_prefix) < 0);
-    if (!message->in_reply_to)
-	message->in_reply_to =
-	    _notmuch_message_get_term (message, i, end, replyto_prefix);
-
-
-    /* It's perfectly valid for a message to have no In-Reply-To
-     * header. For these cases, we return an empty string. */
-    if (!message->in_reply_to)
-	message->in_reply_to = talloc_strdup (message, "");
 }
 
 void
-- 
2.11.0


Thread: