From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id F1C486DE1833 for ; Sat, 11 Mar 2017 16:33:09 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -0.005 X-Spam-Level: X-Spam-Status: No, score=-0.005 tagged_above=-999 required=5 tests=[AWL=0.006, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ugIZU1T0QAvt for ; Sat, 11 Mar 2017 16:33:08 -0800 (PST) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id 251B66DE182E for ; Sat, 11 Mar 2017 16:33:08 -0800 (PST) Received: from remotemail by fethera.tethera.net with local (Exim 4.84_2) (envelope-from ) id 1cmrQv-0004UE-LE; Sat, 11 Mar 2017 19:32:25 -0500 Received: (nullmailer pid 13275 invoked by uid 1000); Sun, 12 Mar 2017 00:33:05 -0000 From: David Bremner To: Steven Allen , notmuch@notmuchmail.org Subject: Re: [bug] notmuch doesn't commit changes before an open transaction on close In-Reply-To: <20151025210215.GA3754@stebalien.com> References: <20151025210215.GA3754@stebalien.com> Date: Sat, 11 Mar 2017 20:33:05 -0400 Message-ID: <871su3qqpq.fsf@tethera.net> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Mar 2017 00:33:10 -0000 Steven Allen 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 (notmuch->xapian_db))->begin_transaction (false); + (static_cast (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());