Date: Mon, 22 Sep 2014 12:00:22 -0700
From: "W. Trevor King" <wking@tremily.us>
To: Austin Clements <amdragon@mit.edu>
Subject: Re: [PATCH] lib: Simplify close and codify aborting atomic section
Message-ID: <20140922190022.GO20827@odin.tremily.us>
References: <1411400615-20934-1-git-send-email-amdragon@mit.edu>
	<20140922165954.GI20827@odin.tremily.us>
	<20140922185050.GB7970@csail.mit.edu>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="5PFZVUeDPxlnBcQp"
Content-Disposition: inline
In-Reply-To: <20140922185050.GB7970@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


--5PFZVUeDPxlnBcQp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Sep 22, 2014 at 06:50:50PM +0000, Austin Clements wrote:
> Quoth W. Trevor King on Sep 22 at  9:59 am:
> > On Mon, Sep 22, 2014 at 11:43:35AM -0400, Austin Clements wrote:
> > > This patch simplifies notmuch_database_close to 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.
> >=20
> > If we're dropping the flush call here, where will it be triggered
> > instead?  We'll need to flush/commit our changes to the database
> > at some point before closing.  Do clients now need an explicit
> > flush/commit command (explicit, client-initiated flushes sound
> > like a good idea to me).
>=20
> The call to Database::close implicitly flushes/commits, as mentioned
> in the comment in the patch, so there's no need for any new APIs or
> client changes.  The call to Database::flush in
> notmuch_database_close was entirely redundant with the call to
> Database::close.

Ah, I thought the implicit flush/commit was just in our code.  Since
it's also in the underlying Xapian close, then this patch looks pretty
good to me.  I'd mention Xapian's explicit close in the notmuch.h
message.  Xapain's docs say [1]:

  For a WritableDatabase, if a transaction is active it will be
  aborted, while if no transaction is active commit() will be
  implicitly called.

Cheers,
Trevor

[1]: http://xapian.org/docs/apidoc/html/classXapian_1_1Database.html#a59f5f=
8b137723dcaaabdbdccbc0cf1eb

--=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

--5PFZVUeDPxlnBcQp
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJUIHHEAAoJEG8/JgBt8ol8TD0P/25sw/RjQN+51URpyT4wAGJg
i89pJ/QBSFz2sk9F/K2GzuOA8LzMqWr1lImQIdi1ypMjHGmmfxkuNWUYWvgQHpc1
KLKaSXql6FAY5KD4mtPGFlLUa9CKNWOqBKwrVRBSjs7UqT8fN+C24XbYA6qnCB0C
Z84TLTxa/4UOpu8nZABASSkWmWcmXphpSdUKzBSLLG6o9iSmaY3YyEXVSfREEV2S
OqmuzOGlxO03hS3j/zOdOrs8CHYrsDvq0kWZZWS7hBIDidZbuXpAsrRJTxpwUScd
o/wjLCfzogyvQhLZZC81LLtACVNkkfhZqES+Bb01cO+/Aoh+GSs0/xn1Qz3kazH1
/IA41Cod+EiE5Wmdx+3D6fK/fkM0NkUcXaE4S6x/AfPYfoeOkKNTBmuA5FjK4N/I
vRiHJAaneUZ6O7wd5xuCe6tgOfXZuNcubdC5p2i9pB+3n03SbzLadBF/ezE8vG2z
srwI37UuPPx/CH1OTtBN8SOab3dbPuuyfI77kbgzhOaPsIsUSiAQutTojEX0p8VE
4rmuv8lfP2gRawO+if7WQUViRSYJrBsLK4U7kl+uEaCKa3uW1rRAXXUtwJfYjwVU
XJwY+wA3pHOibfiFaHcGV0XRMkOQ4XnP8PMBI6NVy9rEjwjuD6wzcpHALYpJcAkj
ZRS7f4VOe+/2kmh3Ot/7
=YbTe
-----END PGP SIGNATURE-----

--5PFZVUeDPxlnBcQp--