On Wed, Sep 7, 2011 at 9:34 PM, Martin Owens <doctormo at gmail.com> wrote: > Hello, > > Per discussions on irc, I've attached a patch for your consideration > which allows the developer to run two new database commands; > > notmuch_database_flush - used on read_write databases to commit changes > to disk > notmuch_database_reopen - used on read_only databases to reload from the > disk > > Used in conjunction they can allow access by multiple programs at the > same time. There are also changes to the python bindings to add these > two methods to the database class. > > Regards, Martin Owens
Ages ago, there was a lot of discussion about improving notmuch's concurrency so that other commands could run simultaneously with long-running operations like notmuch new (whereas currently many commands always abort and others are likely to fail with a concurrent modification exception). This patch, combined with the atomicity changes (assuming they ever get in), would make it possible to implement the concurrency protocol I suggested way back in id:"AANLkTi=KOx8aTJipkiArFVjEHE6zt_JypoASMiiAWBZ6 at mail.gmail.com". A few comments on your patch: --- a/debian/control +++ b/debian/control @@ -5,7 +5,7 @@ Maintainer: Carl Worth <cworth at debian.org> Uploaders: Jameson Graef Rollins <jrollins at finestructure.net>, martin f. krafft <madduck at debian.org>, David Bremner <bremner at debian.org> Build-Depends: debhelper (>= 7.0.50~), pkg-config, libxapian-dev, - libgmime-2.4-dev, libtalloc-dev, libz-dev, python-all (>= 2.6.6-3~), + libgmime-2.4-dev, libtalloc-dev, libz-dev, python-all (>= 2.6.6), Did you mean to change this? --- a/lib/database.cc +++ b/lib/database.cc @@ -700,18 +700,37 @@ notmuch_database_open (const char *path, } void -notmuch_database_close (notmuch_database_t *notmuch) +notmuch_database_reopen (notmuch_database_t *notmuch) { try { - if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE) - (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush (); + if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY) + (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->reopen(); This cast will fail. Shouldn't this just be a wrapper around notmuch->xapian_db->reopen? --- a/lib/query.cc +++ b/lib/query.cc @@ -185,10 +185,14 @@ notmuch_query_search_messages (notmuch_query_t *query) messages->iterator_end = mset.end (); return &messages->base; - + } catch (const Xapian::DatabaseModifiedError &error) { + fprintf (stderr, "Database changed, you need to reopen it before performing queries.\n"); + notmuch->exception_reported = TRUE; + talloc_free (messages); + return NULL; There are many places where DatabaseModifiedError could be thrown besides this function. This needs to be handled more systematically. Also, your patch has a number of code- and patch-formatting problems. Please take a look at the (work-in-progress) patch formatting guidelines at http://notmuchmail.org/patchformatting/ . For the code style, I'd recommend looking at the code around what you're changing. For example, notmuch uses tab indentation and a space before the open paren of an argument list.