lib: autocommit after some number of completed transactions

This change addresses two known issues with large sets of changes to
the database.  The first is that as reported by Steven Allen [1],
notmuch commits are not "flushed" when they complete, which means that
if there is an open transaction when the database closes (or e.g. the
program crashes) then all changes since the last commit will be
discarded (nothing is irrecoverably lost for "notmuch new", as the
indexing process just restarts next time it is run).  This does not
really "fix" the issue reported in [1]; that seems rather difficult
given how transactions work in Xapian. On the other hand, with the
default settings, this should mean one only loses less than a minutes
worth of work.  The second issue is the occasionally reported "storm"
of disk writes when notmuch finishes. I don't yet have a test for
this, but I think committing as we go should reduce the amount of work
when finalizing the database.

[1]: id:20151025210215.GA3754@stebalien.com

diff --git a/lib/database-private.h b/lib/database-private.h
index 1a73dacc365f563134d123fe09d91668b45a6d1f..9706c17e68f9de13fd4377a4386592fe0894e2be 100644 (file)
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -212,6 +212,11 @@ struct _notmuch_database {
     char thread_id_str[17];
     uint64_t last_thread_id;
 
+    /* How many transactions have successfully completed since we last committed */
+    int transaction_count;
+    /* when to commit and reset the counter */
+    int transaction_threshold;
+
     /* error reporting; this value persists only until the
      * next library call. May be NULL */
     char *status_string;

diff --git a/lib/database.cc b/lib/database.cc
index aed053884108fdcf3ce30d94edc78fc77249c92a..31794900a392194e641675b2ec15df3214b075f8 100644 (file)
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1134,13 +1134,21 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch)
     db = notmuch->writable_xapian_db;
     try {
        db->commit_transaction ();
-
-       /* This is a hack for testing.  Xapian never flushes on a
-        * non-flushed commit, even if the flush threshold is 1.
-        * However, we rely on flushing to test atomicity. */
+       notmuch->transaction_count++;
+
+       /* Xapian never flushes on a non-flushed commit, even if the
+        * flush threshold is 1.  However, we rely on flushing to test
+        * atomicity. On the other hand, we can't straight replace
+        * XAPIAN_FLUSH_THRESHOLD with our autocommit counter, because
+        * the former also applies outside notmuch atomic
+        * commits. Hence the follow complicated  test */
        const char *thresh = getenv ("XAPIAN_FLUSH_THRESHOLD");
-       if (thresh && atoi (thresh) == 1)
+       if ((notmuch->transaction_threshold > 0 &&
+            notmuch->transaction_count >= notmuch->transaction_threshold) ||
+           (thresh && atoi (thresh) == 1)) {
            db->commit ();
+           notmuch->transaction_count = 0;
+       }
     } catch (const Xapian::Error &error) {
        _notmuch_database_log (notmuch, "A Xapian exception occurred committing transaction: %s.\n",
                               error.get_msg ().c_str ());

diff --git a/lib/open.cc b/lib/open.cc
index 72f1232c1a990399407680779f2204b7f56a8a30..7b95c5b151d14f958f7b02341cd3eb27229ab285 100644 (file)
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -256,6 +256,8 @@ _alloc_notmuch ()
     notmuch->writable_xapian_db = NULL;
     notmuch->config_path = NULL;
     notmuch->atomic_nesting = 0;
+    notmuch->transaction_count = 0;
+    notmuch->transaction_threshold = 0;
     notmuch->view = 1;
     return notmuch;
 }
@@ -365,6 +367,8 @@ _finish_open (notmuch_database_t *notmuch,
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     char *incompat_features;
     char *message = NULL;
+    const char *autocommit_str;
+    char *autocommit_end;
     unsigned int version;
     const char *database_path = notmuch_database_get_path (notmuch);
 
@@ -461,6 +465,14 @@ _finish_open (notmuch_database_t *notmuch,
        if (status)
            goto DONE;
 
+       autocommit_str = notmuch_config_get (notmuch, NOTMUCH_CONFIG_AUTOCOMMIT);
+       if (unlikely (! autocommit_str)) {
+           INTERNAL_ERROR ("missing configuration for autocommit");
+       }
+       notmuch->transaction_threshold = strtoul (autocommit_str, &autocommit_end, 10);
+       if (*autocommit_end != '\0')
+           INTERNAL_ERROR ("Malformed database database.autocommit value: %s", autocommit_str);
+
        status = _notmuch_database_setup_standard_query_fields (notmuch);
        if (status)
            goto DONE;

diff --git a/test/T385-transactions.sh b/test/T385-transactions.sh
index ebfec2ed4fd253dcacba312244aabfbe6b846dab..d8bb502d61303cc816a92c8e2fe6a4b680279e44 100755 (executable)
--- a/test/T385-transactions.sh
+++ b/test/T385-transactions.sh
@@ -26,7 +26,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Some changes saved with open transaction"
-test_subtest_known_broken
 notmuch config set database.autocommit 1000
 rm -r ${MAIL_DIR}/.notmuch
 notmuch_with_shim no-close new