[PATCH v4] lib: Simplify close and codify aborting atomic section

Austin Clements aclements at csail.mit.edu
Thu Oct 2 13:39:41 PDT 2014


On Thu, 02 Oct 2014, "W. Trevor King" <wking at tremily.us> wrote:
> On Thu, Oct 02, 2014 at 03:19:08PM -0400, Austin Clements wrote:
>> This patch simplifies notmuch_database_close to explicitly abort any
>> outstanding transaction and then just call Database::close.  This
>> works for both read-only and read/write databases, takes care of
>> committing changes, unifies the exception handling path, and codifies
>> aborting outstanding transactions.
>
> I don't expect atomic blocks are particularly useful for read-only
> connections.  If they aren't, I'd quibble with the “This works for
> both read-only…” wording above.  If they are, I'd drop the read/write

It's true that atomic sections aren't very useful on a read-only
database, but we do allow them for symmetry.  We also keep track of how
deeply nested we are so notmuch_database_end_atomic can return a proper
error message regardless of whether the database is read/write or
read-only.

But all I'm saying in the commit message is that Xapian::Database::close
works for both read-only and read/write databases and will flush if
necessary, so we don't have to worry about that.

> check below:
>
>> +	    /* If there's an outstanding transaction, it's unclear if
>> +	     * closing the Xapian database commits everything up to
>> +	     * that transaction, or may discard committed (but
>> +	     * unflushed) transactions.  To be certain, explicitly
>> +	     * cancel any outstanding transaction before closing. */
>> +	    if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE &&
>> +		notmuch->atomic_nesting)
>> +		(static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))
>> +		    ->cancel_transaction ();

The notmuch->mode check here is necessary because atomic_nesting can be
non-zero on a read-only database (for the reason I mentioned above), but
we had better not cast xapian_db to a WritableDatabase if it isn't a
WritableDatabase.

> Thats a very minor quibble though, and I'd be glad to see this patch
> land as is.
>
> Cheers,
> Trevor
>
> -- 
> This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
> For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


More information about the notmuch mailing list