[PATCH 0/3] notmuch-deliver: wait for database to become unlocked and test

[PATCH 0/3] notmuch-deliver: wait for database to become unlocked and test

[David Riebenbauer]

Hi,

I noticed that mail would bounce in my setup, if the xapian database
was locked while notmuch-deliver tried to run.

My solution was to make it wait for some time adn retry. A test
program for is alos included.

Regards,
David

David Riebenbauer (3):
  notmuch-deliver: wait for the database to become unlocked
  notmuch-deliver: test wait for database to become available
  notmuch-deliver: Convert test program to glib main loop.

 contrib/notmuch-deliver/.gitignore          |    2 +
 contrib/notmuch-deliver/Makefile.am         |    2 +-
 contrib/notmuch-deliver/configure.ac        |    1 +
 contrib/notmuch-deliver/src/main.c          |   25 +++-
 contrib/notmuch-deliver/test/Makefile.am    |   26 +++
 contrib/notmuch-deliver/test/nm-test.sh     |    9 +
 contrib/notmuch-deliver/test/nm-testconfig  |   12 ++
 contrib/notmuch-deliver/test/notmuch-lock.c |  226 +++++++++++++++++++++++++++
 contrib/notmuch-deliver/test/testmail       |    7 +
 9 files changed, 306 insertions(+), 4 deletions(-)
 create mode 100644 contrib/notmuch-deliver/test/Makefile.am
 create mode 100755 contrib/notmuch-deliver/test/nm-test.sh
 create mode 100644 contrib/notmuch-deliver/test/nm-testconfig
 create mode 100644 contrib/notmuch-deliver/test/notmuch-lock.c
 create mode 100644 contrib/notmuch-deliver/test/testmail

-- 
1.7.7.1

[PATCH 1/3] notmuch-deliver: wait for the database to become unlocked

[David Riebenbauer]

If notmuch-deliver just gives up on a locked database this can result
in mail bouncing.

This patch makes it wait a given time and then retry to open the
database. The wait time starts at a millisecond, and doubles every
time the database is still locked, until an absolute maximum time of
10 minutes is reached.
---
 contrib/notmuch-deliver/src/main.c |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/contrib/notmuch-deliver/src/main.c b/contrib/notmuch-deliver/src/main.c
index f7a4eaa..e30cbac 100644
--- a/contrib/notmuch-deliver/src/main.c
+++ b/contrib/notmuch-deliver/src/main.c
@@ -71,6 +71,9 @@
 #define EX_CONFIG 78
 #endif
 
+#define MIN_UWAIT 1000        // 1 millisecond
+#define MAX_UWAIT 600000000   // 10 minutes
+
 static gboolean opt_create, opt_fatal, opt_folder, opt_version;
 static gboolean opt_verbose = FALSE;
 static gchar **opt_tags = NULL;
@@ -428,10 +431,26 @@ main(int argc, char **argv)
 		maildir = g_strdup(db_path);
 
 	g_debug("Opening notmuch database `%s'", db_path);
-	db = notmuch_database_open(db_path, NOTMUCH_DATABASE_MODE_READ_WRITE);
+	gint32 wait_time = 0;
+	gint32 next_wait = MIN_UWAIT;
+	while ((db = notmuch_database_open(db_path, NOTMUCH_DATABASE_MODE_READ_WRITE)) == NULL) {
+		g_debug("Retrying to open database in %u microseconds", next_wait);
+		usleep(next_wait);
+
+		next_wait *= 2;
+		if (wait_time + next_wait >= MAX_UWAIT) {
+			wait_time = next_wait - MAX_UWAIT;
+			if (wait_time <= 0) {
+				g_free(db_path);
+				if (db == NULL)
+					return EX_SOFTWARE;
+			}
+		}
+
+		wait_time += next_wait;
+	}
 	g_free(db_path);
-	if (db == NULL)
-		return EX_SOFTWARE;
+
 	if (notmuch_database_needs_upgrade(db)) {
 		g_message("Upgrading database");
 		notmuch_database_upgrade(db, NULL, NULL);
-- 
1.7.7.1

[PATCH 2/3] notmuch-deliver: test wait for database to become available

[David Riebenbauer]

Check if notmuch-deliver really waits for the database to become
available again.

This is done via a test program that locks the notmuch database, and then
spawns the program to be tested. The test can be run with make check.
---
 contrib/notmuch-deliver/.gitignore          |    2 +
 contrib/notmuch-deliver/Makefile.am         |    2 +-
 contrib/notmuch-deliver/configure.ac        |    1 +
 contrib/notmuch-deliver/test/Makefile.am    |   26 ++++
 contrib/notmuch-deliver/test/nm-test.sh     |    9 ++
 contrib/notmuch-deliver/test/nm-testconfig  |   12 ++
 contrib/notmuch-deliver/test/notmuch-lock.c |  167 +++++++++++++++++++++++++++
 contrib/notmuch-deliver/test/testmail       |    7 +
 8 files changed, 225 insertions(+), 1 deletions(-)
 create mode 100644 contrib/notmuch-deliver/test/Makefile.am
 create mode 100755 contrib/notmuch-deliver/test/nm-test.sh
 create mode 100644 contrib/notmuch-deliver/test/nm-testconfig
 create mode 100644 contrib/notmuch-deliver/test/notmuch-lock.c
 create mode 100644 contrib/notmuch-deliver/test/testmail

diff --git a/contrib/notmuch-deliver/.gitignore b/contrib/notmuch-deliver/.gitignore
index 6971ef2..91da18b 100644
--- a/contrib/notmuch-deliver/.gitignore
+++ b/contrib/notmuch-deliver/.gitignore
@@ -40,3 +40,5 @@ m4/lt*.m4
 
 maildrop/numlib/config.h
 src/notmuch-deliver
+test/notmuch-lock
+test/nm-test-mail
diff --git a/contrib/notmuch-deliver/Makefile.am b/contrib/notmuch-deliver/Makefile.am
index 365558a..4392771 100644
--- a/contrib/notmuch-deliver/Makefile.am
+++ b/contrib/notmuch-deliver/Makefile.am
@@ -3,6 +3,6 @@ MAINTAINERCLEANFILES= Makefile.in configure aclocal.m4 \
 		      config.h config.h.in INSTALL
 ACLOCAL_AMFLAGS= -I m4
 AUTOMAKE_OPTIONS= dist-bzip2 no-dist-gzip std-options foreign
-SUBDIRS= maildrop/numlib src .
+SUBDIRS= maildrop/numlib src test .
 
 doc_DATA= README.mkd
diff --git a/contrib/notmuch-deliver/configure.ac b/contrib/notmuch-deliver/configure.ac
index 4deb658..693923f 100644
--- a/contrib/notmuch-deliver/configure.ac
+++ b/contrib/notmuch-deliver/configure.ac
@@ -152,5 +152,6 @@ AC_OUTPUT(
 		  Makefile
 		  maildrop/numlib/Makefile
 		  src/Makefile
+		  test/Makefile
 		  )
 dnl }}}
diff --git a/contrib/notmuch-deliver/test/Makefile.am b/contrib/notmuch-deliver/test/Makefile.am
new file mode 100644
index 0000000..dfa0b45
--- /dev/null
+++ b/contrib/notmuch-deliver/test/Makefile.am
@@ -0,0 +1,26 @@
+DEFS+= -DGITHEAD=\"$(GITHEAD)\"
+AM_CFLAGS= @NOTMUCH_DELIVER_CFLAGS@ $(glib_CFLAGS)
+
+check_PROGRAMS= notmuch-lock
+
+notmuch_lock_SOURCES= notmuch-lock.c
+notmuch_lock_LDADD= $(glib_LIBS)
+
+TESTS = nm-test.sh
+
+nm-test.sh: notmuch-lock \
+            nm-test-mail/INBOX/cur \
+            nm-test-mail/INBOX/new \
+            nm-test-mail/INBOX/tmp
+
+nm-test-mail/INBOX/cur:
+	mkdir -p $@
+
+nm-test-mail/INBOX/new:
+	mkdir -p $@
+
+nm-test-mail/INBOX/tmp:
+	mkdir -p $@
+
+clean-local:
+	rm -rf nm-test-mail
diff --git a/contrib/notmuch-deliver/test/nm-test.sh b/contrib/notmuch-deliver/test/nm-test.sh
new file mode 100755
index 0000000..6113211
--- /dev/null
+++ b/contrib/notmuch-deliver/test/nm-test.sh
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+set -e
+
+export NOTMUCH_CONFIG=./nm-testconfig
+
+notmuch new
+
+./notmuch-lock -s 1000000 -- ../src/notmuch-deliver -v INBOX < testmail
diff --git a/contrib/notmuch-deliver/test/nm-testconfig b/contrib/notmuch-deliver/test/nm-testconfig
new file mode 100644
index 0000000..b94e8d0
--- /dev/null
+++ b/contrib/notmuch-deliver/test/nm-testconfig
@@ -0,0 +1,12 @@
+[database]
+path=./nm-test-mail
+
+[user]
+name=NotMuch Test
+primary_email=notmuc@example.com
+
+[new]
+tags=unread;inbox;
+
+[maildir]
+synchronize_flags=true
diff --git a/contrib/notmuch-deliver/test/notmuch-lock.c b/contrib/notmuch-deliver/test/notmuch-lock.c
new file mode 100644
index 0000000..2303843
--- /dev/null
+++ b/contrib/notmuch-deliver/test/notmuch-lock.c
@@ -0,0 +1,167 @@
+/* vim: set cino=t0 fo=croql sw=4 ts=4 sts=0 noet cin fdm=syntax nolist : */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <glib.h>
+
+#include <notmuch.h>
+
+
+#define MIN_UWAIT 1000        // 1 millisecond
+#define MAX_UWAIT 600000000   // 10 minutes
+
+
+
+static gint sleep_option = -1;
+static GOptionEntry entries[] = {
+	{ "sleep", 's', G_OPTION_FLAG_OPTIONAL_ARG, G_OPTION_ARG_INT, &sleep_option,
+		"Sleep for N microseconds, while holding the lock to the database", "N" },
+	{NULL, 0, 0, 0, NULL, NULL, NULL},
+};
+
+
+
+static gboolean
+load_keyfile(const gchar *path, gchar **db_path, gchar ***tags)
+{
+	GKeyFile *fd;
+	GError *error;
+
+	fd = g_key_file_new();
+	error = NULL;
+	if (!g_key_file_load_from_file(fd, path, G_KEY_FILE_NONE,
+				&error)) {
+		g_printerr("Failed to parse `%s': %s", path,
+				error->message);
+		g_error_free(error);
+		g_key_file_free(fd);
+		return FALSE;
+	}
+
+	*db_path = g_key_file_get_string(fd, "database",
+			"path", &error);
+	if (*db_path == NULL) {
+		g_critical("Failed to parse database.path from `%s': %s", path,
+				error->message);
+		g_error_free(error);
+		g_key_file_free(fd);
+		return FALSE;
+	}
+
+	*tags = g_key_file_get_string_list(fd, "new",
+			"tags", NULL, NULL);
+
+	g_key_file_free(fd);
+	return TRUE;
+}
+
+
+
+static gchar *
+get_db_path(void)
+{
+	gchar *conf_path, *db_path;
+
+	// Get location of notmuch config
+	if (g_getenv("NOTMUCH_CONFIG"))
+		conf_path = g_strdup(g_getenv("NOTMUCH_CONFIG"));
+	else if (g_getenv("HOME"))
+		conf_path = g_build_filename(g_getenv("HOME"),
+				".notmuch-config", NULL);
+	else {
+		g_critical("Neither NOTMUCH_CONFIG nor HOME set");
+		return NULL;
+	}
+
+	// Get location of notmuch database
+	gchar **conf_tags = NULL;
+	g_print("Parsing configuration from `%s'\n", conf_path);
+	if (!load_keyfile(conf_path, &db_path, &conf_tags)) {
+		g_free(conf_path);
+		return NULL;
+	}
+
+	g_free(conf_path);
+	return db_path;
+}
+
+
+int
+main(int argc, char *argv[])
+{
+	gchar *db_path;
+	gint32 sleep_time = 0;
+	notmuch_database_t *db;
+	GError *error = NULL;
+	GOptionContext *context;
+
+	// parse command line options
+	gboolean option_parse_success = FALSE;
+	context = g_option_context_new( "COMMAND [ARGS...]");
+	g_option_context_add_main_entries(context, entries, NULL);
+	g_option_context_set_summary(context,
+			"Utility to test behaviour of programs while the notmuch database is locked");
+	option_parse_success = g_option_context_parse(context, &argc, &argv, &error);
+	g_option_context_free(context);
+	if (! option_parse_success) {
+		g_printerr("option parsing failed: %s\n", error->message);
+		return EXIT_FAILURE;
+	}
+
+	// Check new program args
+	if ((argc - 1) <= 0) {
+		g_printerr("no command supplied\n");
+		return EXIT_FAILURE;
+	}
+	char **new_argv = argv + 1;
+	if (g_strcmp0(new_argv[0], "--") == 0)
+		new_argv++;
+
+	// Get notmuch database path
+	db_path = get_db_path();
+	if (db_path == NULL)
+		return EXIT_FAILURE;
+
+	// Open notmuch database
+	g_printerr("Opening notmuch database `%s'\n", db_path);
+	db = notmuch_database_open(db_path, NOTMUCH_DATABASE_MODE_READ_WRITE);
+	g_free(db_path);
+	if (db == NULL)
+		return EXIT_FAILURE;
+
+	gboolean spawn_success = FALSE;
+	GPid child_pid = 0;
+	spawn_success = g_spawn_async(
+			g_get_current_dir(),
+			new_argv,
+			NULL,
+			G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN | G_SPAWN_DO_NOT_REAP_CHILD,
+			NULL,
+			NULL,
+			&child_pid,
+			NULL
+			);
+	if (!spawn_success) {
+		g_printerr("faild to spawn child\n");
+	}
+
+	// Sleep for some time
+	if (sleep_option >= 0) {
+		sleep_time = sleep_option;
+	} else {
+		sleep_time = g_random_int_range(MIN_UWAIT, MAX_UWAIT);
+	}
+	g_printerr("Sleeping for %f secs\n", ((double)sleep_time)/(1000*1000));
+	usleep(sleep_time);
+
+	// Close database again
+	notmuch_database_close(db);
+
+	int child_status = 0;
+	child_pid = waitpid(child_pid, &child_status, 0);
+
+	return WEXITSTATUS(child_status);
+}
diff --git a/contrib/notmuch-deliver/test/testmail b/contrib/notmuch-deliver/test/testmail
new file mode 100644
index 0000000..264518d
--- /dev/null
+++ b/contrib/notmuch-deliver/test/testmail
@@ -0,0 +1,7 @@
+From: test@example.com
+To: test@example.com
+Subject: testmail
+Message-Id: <20111022050326.653C3201BF@example.com>
+Date: Sat, 22 Oct 2011 07:03:26 +0200 (CEST)
+
+testmail
-- 
1.7.7.1

[PATCH 3/3] notmuch-deliver: Convert test program to glib main loop.

[David Riebenbauer]

This makes reporting of success/failure more robust.
---
 contrib/notmuch-deliver/test/notmuch-lock.c |  103 +++++++++++++++++++++------
 1 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/contrib/notmuch-deliver/test/notmuch-lock.c b/contrib/notmuch-deliver/test/notmuch-lock.c
index 2303843..78e57b6 100644
--- a/contrib/notmuch-deliver/test/notmuch-lock.c
+++ b/contrib/notmuch-deliver/test/notmuch-lock.c
@@ -14,6 +14,19 @@
 #define MAX_UWAIT 600000000   // 10 minutes
 
 
+typedef struct ChildFinishedData_
+{
+	GMainLoop *main_loop;
+	int return_val;
+} ChildFinishedData;
+
+typedef struct SpawnChildData_
+{
+	char** new_argv;
+	ChildFinishedData *child_finidshed_data;
+	GMainLoop *main_loop;
+	int return_val;
+} SpawnChildData;
 
 static gint sleep_option = -1;
 static GOptionEntry entries[] = {
@@ -88,6 +101,65 @@ get_db_path(void)
 	return db_path;
 }
 
+static void
+child_finished_cb(GPid pid, gint status, gpointer data)
+{
+	ChildFinishedData *child_finidshed_data = (ChildFinishedData *)data;
+
+	g_printerr("Called child_finished_cb()\n");
+
+	if WIFEXITED(status) {
+		child_finidshed_data->return_val = WEXITSTATUS(status);
+		g_printerr("PID %d exited normally with exit code %d\n", pid, WEXITSTATUS(status));
+	}
+
+	g_main_loop_quit(child_finidshed_data->main_loop);
+}
+
+
+static gboolean
+spawn_child_cb(gpointer data)
+{
+	SpawnChildData *spawn_child_data = (SpawnChildData *)data;
+
+	g_printerr("Called spawn_child_cb()\n");
+
+	gboolean spawn_success = FALSE;
+	GPid child_pid = 0;
+	spawn_success = g_spawn_async(
+			g_get_current_dir(),
+			spawn_child_data->new_argv,
+			NULL,
+			G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN | G_SPAWN_DO_NOT_REAP_CHILD,
+			NULL,
+			NULL,
+			&child_pid,
+			NULL
+			);
+	if (spawn_success) {
+		g_child_watch_add(child_pid, &child_finished_cb, spawn_child_data->child_finidshed_data);
+	}
+	else {
+		g_printerr("faild to spawn child\n");
+	}
+
+
+	return FALSE;
+}
+
+static gboolean
+close_db_cb(gpointer data)
+{
+	notmuch_database_t *db = (notmuch_database_t*)data;
+
+	g_printerr("Called close_db_cb()\n");
+
+	// Close database again
+	notmuch_database_close(db);
+
+	return FALSE;
+}
+
 
 int
 main(int argc, char *argv[])
@@ -132,22 +204,6 @@ main(int argc, char *argv[])
 	if (db == NULL)
 		return EXIT_FAILURE;
 
-	gboolean spawn_success = FALSE;
-	GPid child_pid = 0;
-	spawn_success = g_spawn_async(
-			g_get_current_dir(),
-			new_argv,
-			NULL,
-			G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN | G_SPAWN_DO_NOT_REAP_CHILD,
-			NULL,
-			NULL,
-			&child_pid,
-			NULL
-			);
-	if (!spawn_success) {
-		g_printerr("faild to spawn child\n");
-	}
-
 	// Sleep for some time
 	if (sleep_option >= 0) {
 		sleep_time = sleep_option;
@@ -155,13 +211,16 @@ main(int argc, char *argv[])
 		sleep_time = g_random_int_range(MIN_UWAIT, MAX_UWAIT);
 	}
 	g_printerr("Sleeping for %f secs\n", ((double)sleep_time)/(1000*1000));
-	usleep(sleep_time);
 
-	// Close database again
-	notmuch_database_close(db);
+	GMainLoop *main_loop = g_main_loop_new(NULL, FALSE);
+
+	ChildFinishedData child_finidshed_data = {main_loop, 0};
+	SpawnChildData spawn_child_data = {new_argv, &child_finidshed_data, main_loop, 0};
+
+	g_idle_add(&spawn_child_cb, &spawn_child_data);
+	g_timeout_add(sleep_time / 1000, &close_db_cb, db);
+	g_main_loop_run(main_loop);
 
-	int child_status = 0;
-	child_pid = waitpid(child_pid, &child_status, 0);
 
-	return WEXITSTATUS(child_status);
+	return child_finidshed_data.return_val;
 }
-- 
1.7.7.1

Re: [PATCH 0/3] notmuch-deliver: wait for database to become unlocked and test

[Ali Polatel]

[-- Attachment #1: Type: text/plain, Size: 2932 bytes --]

On Sun, Nov 06, 2011 at 12:23:01PM +0100, David Riebenbauer wrote:
>Hi,
>
>I noticed that mail would bounce in my setup, if the xapian database
>was locked while notmuch-deliver tried to run.

I recall having this problem sometime ago...

>My solution was to make it wait for some time adn retry. A test
>program for is alos included.

Looks like a workaround more than a solution. What happens if the
database doesn't become available for writing in the meantime?

Quoting from:
http://xapian.org/docs/admin_notes.html#single-writer-multiple-reader

"Xapian enforces this restriction using by having a writer lock the database.
Each Xapian database directory contains a lock file named flintlock (we've kept
the same name as flint used, since the locking technique is the same).

This lock-file will always exist, but will be locked using fcntl() when the
database is open for writing. Because of the semantics of fcntl() locking, for
each WritableDatabase opened we spawn a child process to hold the lock, which
then exec-s cat, so you will see a cat subprocess of any writer process in the
output of ps, top, etc."

See? It's a simple fcntl() lock!

That said, I'm against adding this kind of kludge to notmuch-deliver or
any other tool accessing the database via libnotmuch. Until the issue is
fixed properly I, and I suppose you, can live with a simple shell script
using flock(1) on the file .notmuch/xapian/flintlock.

One possible solution is described by Austin in the mail:
id:AANLkTi=KOx8aTJipkiArFVjEHE6zt_JypoASMiiAWBZ6@mail.gmail.com
which I quite like.

>Regards,
>David

		-alip

>David Riebenbauer (3):
>  notmuch-deliver: wait for the database to become unlocked
>  notmuch-deliver: test wait for database to become available
>  notmuch-deliver: Convert test program to glib main loop.
>
> contrib/notmuch-deliver/.gitignore          |    2 +
> contrib/notmuch-deliver/Makefile.am         |    2 +-
> contrib/notmuch-deliver/configure.ac        |    1 +
> contrib/notmuch-deliver/src/main.c          |   25 +++-
> contrib/notmuch-deliver/test/Makefile.am    |   26 +++
> contrib/notmuch-deliver/test/nm-test.sh     |    9 +
> contrib/notmuch-deliver/test/nm-testconfig  |   12 ++
> contrib/notmuch-deliver/test/notmuch-lock.c |  226 +++++++++++++++++++++++++++
> contrib/notmuch-deliver/test/testmail       |    7 +
> 9 files changed, 306 insertions(+), 4 deletions(-)
> create mode 100644 contrib/notmuch-deliver/test/Makefile.am
> create mode 100755 contrib/notmuch-deliver/test/nm-test.sh
> create mode 100644 contrib/notmuch-deliver/test/nm-testconfig
> create mode 100644 contrib/notmuch-deliver/test/notmuch-lock.c
> create mode 100644 contrib/notmuch-deliver/test/testmail
>
>-- 
>1.7.7.1
>
>_______________________________________________
>notmuch mailing list
>notmuch@notmuchmail.org
>http://notmuchmail.org/mailman/listinfo/notmuch

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]