From 0c4a7a6004ce81dae5785ff8516e8566d9c6af47 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 3 Oct 2014 16:39:41 +2000 Subject: [PATCH] Re: [PATCH v4] lib: Simplify close and codify aborting atomic section --- 17/4c4cafd68a82ec87bbfdd63a0e2ce9f69183d2 | 104 ++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 17/4c4cafd68a82ec87bbfdd63a0e2ce9f69183d2 diff --git a/17/4c4cafd68a82ec87bbfdd63a0e2ce9f69183d2 b/17/4c4cafd68a82ec87bbfdd63a0e2ce9f69183d2 new file mode 100644 index 000000000..4625354f2 --- /dev/null +++ b/17/4c4cafd68a82ec87bbfdd63a0e2ce9f69183d2 @@ -0,0 +1,104 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 1FAD1431FBC + for ; Thu, 2 Oct 2014 13:39:53 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -2.3 +X-Spam-Level: +X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id zUY7V3CUtcgU for ; + Thu, 2 Oct 2014 13:39:45 -0700 (PDT) +Received: from outgoing.csail.mit.edu (outgoing.csail.mit.edu [128.30.2.149]) + by olra.theworths.org (Postfix) with ESMTP id EC8B7431FB6 + for ; Thu, 2 Oct 2014 13:39:44 -0700 (PDT) +Received: from [104.131.20.129] (helo=awakeningjr) + by outgoing.csail.mit.edu with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) + (Exim 4.72) (envelope-from ) + id 1XZnA6-0001qT-4v; Thu, 02 Oct 2014 16:39:42 -0400 +Received: from amthrax by awakeningjr with local (Exim 4.84) + (envelope-from ) + id 1XZnA5-000627-Oa; Thu, 02 Oct 2014 16:39:41 -0400 +From: Austin Clements +To: "W. Trevor King" +Subject: Re: [PATCH v4] lib: Simplify close and codify aborting atomic section +In-Reply-To: <20141002194134.GP3770@odin.tremily.us> +References: <87mw9ee0vf.fsf@csail.mit.edu> + <1412277548-3445-1-git-send-email-aclements@csail.mit.edu> + <20141002194134.GP3770@odin.tremily.us> +User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/24.3.1 + (x86_64-pc-linux-gnu) +Date: Thu, 02 Oct 2014 16:39:41 -0400 +Message-ID: <87k34idx4i.fsf@csail.mit.edu> +MIME-Version: 1.0 +Content-Type: text/plain; charset=utf-8 +Content-Transfer-Encoding: quoted-printable +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +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: Thu, 02 Oct 2014 20:39:53 -0000 + +On Thu, 02 Oct 2014, "W. Trevor King" 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 =E2=80=9CThis works for +> both read-only=E2=80=A6=E2=80=9D wording above. If they are, I'd drop th= +e 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 =3D=3D NOTMUCH_DATABASE_MODE_READ_WRITE && +>> + notmuch->atomic_nesting) +>> + (static_cast (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 +> +> --=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 -- 2.26.2