Re: Patch: Flush and Reopen
authorAustin Clements <amdragon@mit.edu>
Fri, 9 Sep 2011 02:42:32 +0000 (22:42 +2000)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:39:24 +0000 (09:39 -0800)
99/344023cfa635bc86d68824e6bd6c01079f3077 [new file with mode: 0644]

diff --git a/99/344023cfa635bc86d68824e6bd6c01079f3077 b/99/344023cfa635bc86d68824e6bd6c01079f3077
new file mode 100644 (file)
index 0000000..8e70226
--- /dev/null
@@ -0,0 +1,144 @@
+Return-Path: <amdragon@gmail.com>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id BC2CC429E26\r
+       for <notmuch@notmuchmail.org>; Thu,  8 Sep 2011 19:42:37 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.699\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5\r
+       tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001,\r
+       RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id ei6CnuOMjWTA for <notmuch@notmuchmail.org>;\r
+       Thu,  8 Sep 2011 19:42:37 -0700 (PDT)\r
+Received: from mail-qy0-f174.google.com (mail-qy0-f174.google.com\r
+       [209.85.216.174]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id F4012431FD0\r
+       for <notmuch@notmuchmail.org>; Thu,  8 Sep 2011 19:42:36 -0700 (PDT)\r
+Received: by qyk7 with SMTP id 7so896065qyk.5\r
+       for <notmuch@notmuchmail.org>; Thu, 08 Sep 2011 19:42:32 -0700 (PDT)\r
+DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;\r
+       h=mime-version:sender:in-reply-to:references:date\r
+       :x-google-sender-auth:message-id:subject:from:to:cc:content-type;\r
+       bh=lf2TaXP2nZAoIf286C2Onf9t2F/+PV6xBmEjsI3oGpw=;\r
+       b=OpJ1N3UELhMMSTTNUovMEihCWxBfQQuSpwQFBEaNwS9gFUqxK7o5qJRkh2KrD4sINx\r
+       6oC0iJ5uUoVLfeciEvZS5jg7zHTUszpk4D5NQnenrObMFRMrXJeBXmRu9zePMIvwWrmB\r
+       1t+O8B1FSUb1DgqB9tgCSCePgkTF5W3in83ds=\r
+MIME-Version: 1.0\r
+Received: by 10.229.100.213 with SMTP id z21mr1085242qcn.102.1315536152616;\r
+       Thu, 08 Sep 2011 19:42:32 -0700 (PDT)\r
+Sender: amdragon@gmail.com\r
+Received: by 10.229.2.201 with HTTP; Thu, 8 Sep 2011 19:42:32 -0700 (PDT)\r
+In-Reply-To: <1315445674.32058.183.camel@delen>\r
+References: <1315445674.32058.183.camel@delen>\r
+Date: Thu, 8 Sep 2011 22:42:32 -0400\r
+X-Google-Sender-Auth: m4ikT1hSvmTZ1jcWgxtuVTIXAYA\r
+Message-ID:\r
+ <CAH-f9WtuYYuksfTHL1ZMmoRF6kyL7iFu9BXDzGn-wWGniSvf9w@mail.gmail.com>\r
+Subject: Re: Patch: Flush and Reopen\r
+From: Austin Clements <amdragon@mit.edu>\r
+To: Martin Owens <doctormo@gmail.com>\r
+Content-Type: text/plain; charset=ISO-8859-1\r
+Cc: Paul Tagliamonte <paultag@ubuntu.com>,\r
+       Notmuch developer list <notmuch@notmuchmail.org>\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Fri, 09 Sep 2011 02:42:37 -0000\r
+\r
+On Wed, Sep 7, 2011 at 9:34 PM, Martin Owens <doctormo@gmail.com> wrote:\r
+> Hello,\r
+>\r
+> Per discussions on irc, I've attached a patch for your consideration\r
+> which allows the developer to run two new database commands;\r
+>\r
+> notmuch_database_flush - used on read_write databases to commit changes\r
+> to disk\r
+> notmuch_database_reopen - used on read_only databases to reload from the\r
+> disk\r
+>\r
+> Used in conjunction they can allow access by multiple programs at the\r
+> same time. There are also changes to the python bindings to add these\r
+> two methods to the database class.\r
+>\r
+> Regards, Martin Owens\r
+\r
+Ages ago, there was a lot of discussion about improving notmuch's\r
+concurrency so that other commands could run simultaneously with\r
+long-running operations like notmuch new (whereas currently many\r
+commands always abort and others are likely to fail with a concurrent\r
+modification exception).  This patch, combined with the atomicity\r
+changes (assuming they ever get in), would make it possible to\r
+implement the concurrency protocol I suggested way back in\r
+id:"AANLkTi=KOx8aTJipkiArFVjEHE6zt_JypoASMiiAWBZ6@mail.gmail.com".\r
+\r
+A few comments on your patch:\r
+\r
+--- a/debian/control\r
++++ b/debian/control\r
+@@ -5,7 +5,7 @@ Maintainer: Carl Worth <cworth@debian.org>\r
+ Uploaders: Jameson Graef Rollins <jrollins@finestructure.net>, martin\r
+f. krafft <madduck@debian.org>,\r
+          David Bremner <bremner@debian.org>\r
+ Build-Depends: debhelper (>= 7.0.50~), pkg-config, libxapian-dev,\r
+- libgmime-2.4-dev, libtalloc-dev, libz-dev,  python-all (>= 2.6.6-3~),\r
++ libgmime-2.4-dev, libtalloc-dev, libz-dev,  python-all (>= 2.6.6),\r
+\r
+Did you mean to change this?\r
+\r
+--- a/lib/database.cc\r
++++ b/lib/database.cc\r
+@@ -700,18 +700,37 @@ notmuch_database_open (const char *path,\r
+ }\r
+\r
+ void\r
+-notmuch_database_close (notmuch_database_t *notmuch)\r
++notmuch_database_reopen (notmuch_database_t *notmuch)\r
+ {\r
+     try {\r
+-      if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)\r
+-          (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush ();\r
++        if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)\r
++            (static_cast <Xapian::WritableDatabase *>\r
+(notmuch->xapian_db))->reopen();\r
+\r
+This cast will fail.  Shouldn't this just be a wrapper around\r
+notmuch->xapian_db->reopen?\r
+\r
+--- a/lib/query.cc\r
++++ b/lib/query.cc\r
+@@ -185,10 +185,14 @@ notmuch_query_search_messages (notmuch_query_t *query)\r
+       messages->iterator_end = mset.end ();\r
+\r
+       return &messages->base;\r
+-\r
++    } catch (const Xapian::DatabaseModifiedError &error) {\r
++      fprintf (stderr, "Database changed, you need to reopen it\r
+before performing queries.\n");\r
++      notmuch->exception_reported = TRUE;\r
++      talloc_free (messages);\r
++      return NULL;\r
+\r
+There are many places where DatabaseModifiedError could be thrown\r
+besides this function.  This needs to be handled more systematically.\r
+\r
+Also, your patch has a number of code- and patch-formatting problems.\r
+Please take a look at the (work-in-progress) patch formatting\r
+guidelines at http://notmuchmail.org/patchformatting/ .  For the code\r
+style, I'd recommend looking at the code around what you're changing.\r
+For example, notmuch uses tab indentation and a space before the open\r
+paren of an argument list.\r