Re: [bug] notmuch doesn't commit changes before an open transaction on close

[David Bremner <david@tethera.net>]

Steven Allen <steven@stebalien.com> writes:

> Notmuch claims to commit changes made before an open transaction on
> close but actually throws them away (according to the documentation).
>
> According to the notmuch documentation,
>
>> For writable databases, notmuch_database_close commits all changes
>> to disk before closing the database.  If the caller is currently in
>> an atomic section (there was a notmuch_database_begin_atomic
>> without a matching notmuch_database_end_atomic), this will discard
>> changes made in that atomic section (but still commit changes made
>> prior to entering the atomic section).
>
> However, this isn't true. Notmuch atomic transactions don't flush on
> entry so, this comment from the xapian documentation applies:
>
>> If you're applying atomic groups of changes and only wish to ensure
>> that each group is either applied or not applied, then you can prevent
>> the automatic commit() before and after the transaction by starting
>> the transaction with begin_transaction(false). However, if
>> cancel_transaction is called (or if commit_transaction isn't called
>> before the WritableDatabase object is destroyed) then any changes
>> which were pending before the transaction began will also be
>> discarded.
>
> source: http://xapian.org/docs/apidoc/html/classXapian_1_1WritableDatabase.html
>
> This means that, in theory at least, xapian could throw away *all*
> changes to the database if a transaction is open.
>

I was curioius what would happen if we did commit on entry, but the
performance loss is pretty catastrophic. The small performance test
corpus runs 20 times slower, and does about 200 times as many writes, so
it would probably be even worse on a machine without SSD.  Perhaps we
should just provide notmuch_database_commit and let people call that,
although it might be just as easy to wrap whatever changes are being
committed in a begin/end atomic.


[1]:

diff --git a/lib/database.cc b/lib/database.cc
index a679cbab..da67a5df 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1720,7 +1720,7 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch)
        return NOTMUCH_STATUS_UPGRADE_REQUIRED;
 
     try {
-       (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false);
+       (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (true);
     } catch (const Xapian::Error &error) {
        _notmuch_database_log (notmuch, "A Xapian exception occurred beginning transaction: %s.\n",
                 error.get_msg().c_str());