Date: Thu, 2 Oct 2014 12:41:34 -0700
From: "W. Trevor King" <wking@tremily.us>
To: Austin Clements <aclements@csail.mit.edu>
Subject: Re: [PATCH v4] lib: Simplify close and codify aborting atomic section
Message-ID: <20141002194134.GP3770@odin.tremily.us>
References: <87mw9ee0vf.fsf@csail.mit.edu>
	<1412277548-3445-1-git-send-email-aclements@csail.mit.edu>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="o6HUWsywbRjTg5uW"
Content-Disposition: inline
In-Reply-To: <1412277548-3445-1-git-send-email-aclements@csail.mit.edu>
OpenPGP: id=39A2F3FA2AB17E5D8764F388FC29BDCDF15F5BE8;
	url=http://tremily.us/pubkey.txt
User-Agent: Mutt/1.5.23 (2014-03-12)
Cc: notmuch@notmuchmail.org
Precedence: list


--o6HUWsywbRjTg5uW
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 =E2=80=9CThis works for
both read-only=E2=80=A6=E2=80=9D wording above.  If they are, I'd drop the =
read/write
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 =3D=3D NOTMUCH_DATABASE_MODE_READ_WRITE &&
> +		notmuch->atomic_nesting)
> +		(static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))
> +		    ->cancel_transaction ();

Thats a very minor quibble though, and I'd be glad to see this patch
land as is.

Cheers,
Trevor

--=20
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

--o6HUWsywbRjTg5uW
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJULapsAAoJEG8/JgBt8ol8GN4QAOFz/qxrPe+yBpxImBYdcwFC
/pVN73sS+1n8nlNtZn+r7HLIJZIvSd56jKTeE9+kopLZ2wqUiyr8xT8V2iY5oUu6
sVLP/YV6hxqeu+bkT/kWhzAZz2VFKdnKeKWL/Oz89ZUDH6PFAyJxcA0eID12+Ojw
EnqjCSRqXv1T4vCFsMWEsFN4mwW3yC258Wgoa3xZwOsxR7eQ+bxKfP7J+hhPj/Tt
7Vj5wP6bPojjSol+r15uaeFthSE5ZN56myQvvybUHMvGMvAgOT/0N8MS5UwOhmtW
BRtvVs2kmCDrVKLYVdb8R5pp28ClF4yj9PlQkR5lE829t6fJnyu91w8aen5FZ7vw
rqrgv5b8N931IxNEG3mcN5MH4lrohUVWWxenqyvqCJz1ijp3bjJIOszhqGv+oC6V
5PVDaFP8oIjsmHjf4V/w0EzhTtzdg+7vXCTmQIxrBUhDwRudmAMCmyEhQAHFDzoY
VxwZBaEbrYdoI4tkbW5gelfXS722gZa8CnmAEYQ7yKPOtTIgaEk1kP+sjTakNh1q
sBHS8kezjwiAeH4BbqEOyo273f2vv1iI3z7od/tCDLF6k1miFXUURs+0qGsYglvs
AX9yh3YgtxqQbaIT6w2LbB6djbshGTILBW27F6trm4ci6w7gkqxC5x0jo+b/X7YP
PLFsWNT3UESGIV2BYsj4
=aGWW
-----END PGP SIGNATURE-----

--o6HUWsywbRjTg5uW--