Re: Patch: Flush and Reopen
On Fri, 2011-09-09 at 08:06 -0300, David Bremner wrote:
I was also puzzled by your changes to debian/changelog. We really need
justification for every little change introduced by the patch; this is
another reason it helps to split things up a bit.
That probably was unintended. See attached for the all new slimmed down
patch.
I've removed the debian control change, the changelog change, the extra
error catch in query.cc and put that cast right.
I'll leave the version of python-all for someone else.
Best Regards, Martin Owens
From 981b91e9405de2daf35d30d9bbeab0dd62b15683 Mon Sep 17 00:00:00 2001
From: Martin Owens docto...@gmail.com
Date: Fri, 9 Sep 2011 13:51:59 -0400
Subject: [PATCH] Add flush and reopen methods to the libnotmuch and to the python bindings. Flush allows read_write databases to commit their changes, forcing a flush to disk and reopen allows a read_only database to reread from the disk.
Also added a handler for Xapian DatabaseChangedError which occurs when the flush happens to read only databases.
---
bindings/python/notmuch/database.py | 10 ++
debian/libnotmuch1.symbols |2 ++
lib/database.cc | 33 ++---
lib/notmuch.h |8
4 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index f18ca14..b6c1c31 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -432,6 +432,16 @@ class Database(object):
return Query(self, querystring)
+def reopen(self):
+Reopens a read only database, when the data has changed, this is required.
+if self._db is not None:
+nmlib.notmuch_database_reopen(self._db)
+
+def flush(self):
+Flushes the search database, only available when in read-write.
+if self._db is not None:
+nmlib.notmuch_database_flush(self._db)
+
def __repr__(self):
return 'Notmuch DB + self.get_path() + '
diff --git a/debian/libnotmuch1.symbols b/debian/libnotmuch1.symbols
index 05d86e6..7e42b0e 100644
--- a/debian/libnotmuch1.symbols
+++ b/debian/libnotmuch1.symbols
@@ -3,6 +3,7 @@ libnotmuch.so.1 libnotmuch1 #MINVER#
notmuch_database_close@Base 0.3
notmuch_database_create@Base 0.3
notmuch_database_find_message@Base 0.3
+ notmuch_database_flush@Base 0.3
notmuch_database_get_all_tags@Base 0.3
notmuch_database_get_directory@Base 0.3
notmuch_database_get_path@Base 0.3
@@ -10,6 +11,7 @@ libnotmuch.so.1 libnotmuch1 #MINVER#
notmuch_database_needs_upgrade@Base 0.3
notmuch_database_open@Base 0.3
notmuch_database_remove_message@Base 0.3
+ notmuch_database_reopen@Base 0.3
notmuch_database_upgrade@Base 0.3
notmuch_directory_destroy@Base 0.3
notmuch_directory_get_child_directories@Base 0.3
diff --git a/lib/database.cc b/lib/database.cc
index 9c2f4ec..b86bf1a 100644
--- 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::Database * (notmuch-xapian_db))-reopen();
} catch (const Xapian::Error error) {
- if (! notmuch-exception_reported) {
- fprintf (stderr, Error: A Xapian exception occurred flushing database: %s\n,
- error.get_msg().c_str());
- }
+if (! notmuch-exception_reported) {
+fprintf (stderr, Error: A Xapian exception occurred reopening database: %s\n,
+error.get_msg().c_str());
+}
}
+}
+void
+notmuch_database_flush (notmuch_database_t *notmuch)
+{
+try {
+if (notmuch-mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
+(static_cast Xapian::WritableDatabase * (notmuch-xapian_db))-flush();
+} catch (const Xapian::Error error) {
+if (! notmuch-exception_reported) {
+fprintf (stderr, Error: A Xapian exception occurred flushing database: %s\n,
+error.get_msg().c_str());
+}
+}
+}
+
+void
+notmuch_database_close (notmuch_database_t *notmuch)
+{
+notmuch_database_flush (notmuch);
delete notmuch-term_gen;
delete notmuch-query_parser;
delete notmuch-xapian_db;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 974be8d..ca87b89 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -171,6 +171,14 @@ notmuch_database_t *
notmuch_database_open (const char *path,
notmuch_database_mode_t mode);
+/* Reopen a read_only database when the data source has changed */
+void
+notmuch_database_reopen (notmuch_database_t *database);
+
+/* Force a flush of the xapian database, also done on close */
+void
Re: Patch: Flush and Reopen
On Fri, Sep 9, 2011 at 1:55 PM, Martin Owens docto...@gmail.com wrote:
That probably was unintended. See attached for the all new slimmed down
patch.
--- 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::Database * (notmuch-xapian_db))-reopen();
I was thinking this should probably just be
try {
notmuch-xapian_db-reopen ();
} catch ...
(indented correctly, of course). Reopen is a method of
Xapian::Database, which is what notmuch-xapian_db is anyway (unlike,
flush, which is a member of the subclass WritableDatabase and hence
requires the cast). And reopening is a sensible thing to do on both
read-only and read/write databases.
Also, in keeping with notmuch code style, you should use tabs for
indentation and put a space before the open paren argument lists.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: Patch: Flush and Reopen
On Fri, 2011-09-09 at 19:40 -0400, Austin Clements wrote:
(indented correctly, of course). Reopen is a method of
Xapian::Database, which is what notmuch-xapian_db is anyway (unlike,
flush, which is a member of the subclass WritableDatabase and hence
requires the cast). And reopening is a sensible thing to do on both
read-only and read/write databases.
See attached for the removal of the cast and conditional for reopen.
Also, in keeping with notmuch code style, you should use tabs for
indentation and put a space before the open paren argument lists.
I couldn't detect a consistant style for the tab/spacing and it confused
me greatly. Especially coming from python where spacing is critical.
I've put in tabs, not sure if I've put in enough in the form required.
Best Regards, Martin Owens
From 0d7180fbf271d696e2105420db87aa2d1f0e72aa Mon Sep 17 00:00:00 2001
From: Martin Owens docto...@gmail.com
Date: Fri, 9 Sep 2011 20:40:21 -0400
Subject: [PATCH] Add flush and reopen methods to the libnotmuch and to the python bindings. Flush allows read_write databases to commit their changes, forcing a flush to disk and reopen allows a read_only database to reread from the disk.
Also added a handler for Xapian DatabaseChangedError which occurs when the flush happens to read only databases.
---
bindings/python/notmuch/database.py | 10 ++
debian/changelog|6 +-
debian/control |2 +-
debian/libnotmuch1.symbols |2 ++
lib/database.cc | 30 --
lib/notmuch.h |8
6 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index f18ca14..b6c1c31 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -432,6 +432,16 @@ class Database(object):
return Query(self, querystring)
+def reopen(self):
+Reopens a read only database, when the data has changed, this is required.
+if self._db is not None:
+nmlib.notmuch_database_reopen(self._db)
+
+def flush(self):
+Flushes the search database, only available when in read-write.
+if self._db is not None:
+nmlib.notmuch_database_flush(self._db)
+
def __repr__(self):
return 'Notmuch DB + self.get_path() + '
diff --git a/debian/changelog b/debian/changelog
index c2b7552..565edfb 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,12 @@
notmuch (0.8~rc1-1) experimental; urgency=low
+ [ David Bremner ]
* Upstream release candidate.
- -- David Bremner brem...@debian.org Tue, 06 Sep 2011 22:24:24 -0300
+ [ Martin Owens (DoctorMO) ]
+ * Re-new-world-order
+
+ -- Martin Owens (DoctorMO) docto...@gmail.com Wed, 07 Sep 2011 18:19:50 -0400
notmuch (0.7-1) unstable; urgency=low
diff --git a/debian/control b/debian/control
index 03afdf4..a72fe1b 100644
--- a/debian/control
+++ b/debian/control
@@ -5,7 +5,7 @@ Maintainer: Carl Worth cwo...@debian.org
Uploaders: Jameson Graef Rollins jroll...@finestructure.net, martin f. krafft madd...@debian.org,
David Bremner brem...@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),
emacs23-nox | emacs23 (=23~) | emacs23-lucid (=23~)
Standards-Version: 3.9.2
Homepage: http://notmuchmail.org/
diff --git a/debian/libnotmuch1.symbols b/debian/libnotmuch1.symbols
index 05d86e6..7e42b0e 100644
--- a/debian/libnotmuch1.symbols
+++ b/debian/libnotmuch1.symbols
@@ -3,6 +3,7 @@ libnotmuch.so.1 libnotmuch1 #MINVER#
notmuch_database_close@Base 0.3
notmuch_database_create@Base 0.3
notmuch_database_find_message@Base 0.3
+ notmuch_database_flush@Base 0.3
notmuch_database_get_all_tags@Base 0.3
notmuch_database_get_directory@Base 0.3
notmuch_database_get_path@Base 0.3
@@ -10,6 +11,7 @@ libnotmuch.so.1 libnotmuch1 #MINVER#
notmuch_database_needs_upgrade@Base 0.3
notmuch_database_open@Base 0.3
notmuch_database_remove_message@Base 0.3
+ notmuch_database_reopen@Base 0.3
notmuch_database_upgrade@Base 0.3
notmuch_directory_destroy@Base 0.3
notmuch_directory_get_child_directories@Base 0.3
diff --git a/lib/database.cc b/lib/database.cc
index 9c2f4ec..557c683 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -700,18 +700,36 @@ notmuch_database_open (const char *path,
}
void
-notmuch_database_close (notmuch_database_t *notmuch)
+notmuch_database_reopen (notmuch_database_t *notmuch)
+{
+ try {
+ notmuch-xapian_db-reopen ();
+ } catch (const Xapian::Error error) {
+ if (! notmuch-exception_reported) {
+ fprintf (stderr, Error: A Xapian exception occurred reopening database: %s\n,
+ error.get_msg().c_str());
+ }
+ }
+}
+
+void
+notmuch_database_flush (notmuch_database_t *notmuch)
Re: Patch: Flush and Reopen
On Wed, Sep 7, 2011 at 9:34 PM, Martin Owens docto...@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_jypoasmiiaw...@mail.gmail.com.
A few comments on your patch:
--- a/debian/control
+++ b/debian/control
@@ -5,7 +5,7 @@ Maintainer: Carl Worth cwo...@debian.org
Uploaders: Jameson Graef Rollins jroll...@finestructure.net, martin
f. krafft madd...@debian.org,
David Bremner brem...@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.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
