[PATCH] Calls to notmuch get queued and executed asynchronously.

[PATCH] Calls to notmuch get queued and executed asynchronously.

[James Vasile]

Added notmuch-enqueue-asynch to replace calls to
notmuch-call-notmuch-process.  Calls to notmuch are then queued and
executed asynchronously.  If the db is busy and we get an error saying
it was locked, keep trying until the db is no longer busy.  Errors go
in a buffer as per usual.

The only caveat here is that if the db is permanently locked (i.e. the
lock is broken), we just keep on trying forever.  Maybe there should
probably be a maximum number of tries or a timeout, but since 'notmuch
new' can take a long time, it's difficult to come up with a reasonable
limit.
---
 notmuch.el |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 6482170..7fc63e9 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -302,7 +302,7 @@ pseudoheader summary"
   "Add a tag to the current message."
   (interactive
    (list (notmuch-select-tag-with-completion "Tag to add: ")))
-  (apply 'notmuch-call-notmuch-process
+  (apply 'notmuch-enqueue-asynch
 	 (append (cons "tag"
 		       (mapcar (lambda (s) (concat "+" s)) toadd))
 		 (cons (notmuch-show-get-message-id) nil)))
@@ -315,7 +315,7 @@ pseudoheader summary"
   (let ((tags (notmuch-show-get-tags)))
     (if (intersection tags toremove :test 'string=)
 	(progn
-	  (apply 'notmuch-call-notmuch-process
+	  (apply 'notmuch-enqueue-asynch
 		 (append (cons "tag"
 			       (mapcar (lambda (s) (concat "-" s)) toremove))
 			 (cons (notmuch-show-get-message-id) nil)))
@@ -1374,6 +1374,53 @@ Complete list of currently available key bindings:
   (let ((message-id (notmuch-search-find-thread-id)))
     (notmuch-reply message-id)))
 
+(defun join-string-list (string-list)
+    "Concatenates a list of strings and puts spaces between the
+elements."
+    (mapconcat 'identity string-list " "))
+
+(defvar notmuch-asynch-queue nil)
+(defun notmuch-call-notmuch-process-asynch (&rest args)
+  "Asynchronously invoke \"notmuch\" with the given list of arguments.
+
+Error output from the process will be presented to the user as an
+error and will also appear in a buffer named \"*notmuch <arguments>*\"."
+  (when args
+    (let ((process-connection-type nil)
+	  (buffer-name (format "*notmuch %s*" (join-string-list args))))
+      (when (get-buffer buffer-name)
+	(kill-buffer (get-buffer buffer-name)))
+      (let* ((process-buffer (get-buffer-create buffer-name))
+	     (process (apply 'start-process "notmuch-process" process-buffer
+			     notmuch-command args)))
+	(set-process-sentinel process 'notmuch-call-notmuch-process-asynch-sentinel)))))
+(defun notmuch-enqueue-asynch (&rest args)
+  "Add a call to notmuch to the queue of notmuch calls.
+
+args is a list of arguments to notmuch.  ex: (\"tag\" \"+list\"
+\"to:mylist@example.com\")
+
+Calls to notmuch are queued and called asynchronously."
+  (setq notmuch-asynch-queue (append notmuch-asynch-queue (list args)))
+  (when (= (length notmuch-asynch-queue) 1)
+    (apply 'notmuch-call-notmuch-process-asynch (pop notmuch-asynch-queue))))
+  
+(defun notmuch-call-notmuch-process-asynch-sentinel (process event)
+  "Handle the exit of a notmuch asynch process.
+
+When notmuch is done processing, display the error or kill the
+error buffer.  If the db was busy on the last attempt to execute
+command, try it again."
+  (with-current-buffer (process-buffer process)
+    (goto-char (point-min))
+    (if (= (process-exit-status process) 0)
+	(kill-buffer (buffer-name (process-buffer process)))
+	(if (search-forward "Unable to acquire database write lock" nil t)
+	    (apply 'notmuch-call-notmuch-process-asynch (cdr (process-command process)))
+	    (error (format "%s: %s" (join-string-list (process-command process))
+			   (buffer-string))))))
+  (apply 'notmuch-call-notmuch-process-asynch (pop notmuch-asynch-queue)))
+
 (defun notmuch-call-notmuch-process (&rest args)
   "Synchronously invoke \"notmuch\" with the given list of arguments.
 
@@ -1420,7 +1467,7 @@ The tag is added to messages in the currently selected thread
 which match the current search terms."
   (interactive
    (list (notmuch-select-tag-with-completion "Tag to add: ")))
-  (notmuch-call-notmuch-process "tag" (concat "+" tag) (notmuch-search-find-thread-id))
+  (notmuch-enqueue-asynch "tag" (concat "+" tag) (notmuch-search-find-thread-id))
   (notmuch-search-set-tags (delete-dups (sort (cons tag (notmuch-search-get-tags)) 'string<))))
 
 (defun notmuch-search-remove-tag (tag)
@@ -1430,7 +1477,7 @@ The tag is removed from messages in the currently selected thread
 which match the current search terms."
   (interactive
    (list (notmuch-select-tag-with-completion "Tag to remove: " (notmuch-search-find-thread-id))))
-  (notmuch-call-notmuch-process "tag" (concat "-" tag) (notmuch-search-find-thread-id))
+  (notmuch-enqueue-asynch "tag" (concat "-" tag) (notmuch-search-find-thread-id))
   (notmuch-search-set-tags (delete tag (notmuch-search-get-tags))))
 
 (defun notmuch-search-archive-thread ()
@@ -1511,7 +1558,7 @@ characters as well as `_.+-'.
 	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
 	  (error "Action must be of the form `+thistag -that_tag'"))
 	(setq words (cdr words))))
-    (apply 'notmuch-call-notmuch-process "tag"
+    (apply 'notmuch-enqueue-asynch "tag"
 	   (append action-split (list notmuch-search-query-string) nil))))
 
 ;;;###autoload
-- 
1.6.3.3

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[James Vasile]

Sleep between retrying asynchronous notmuch commands and check for
notmuch-process before firing off a new one

If the db is locked when notmuch tries to write to it, an error is
reported to the client (in this case, notmuch.el).  Instead of
accepting that error, wait a small amount of time and then try again.

Also, don't start multiple asynchronous processes.
---
 notmuch.el |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 7fc63e9..31e89b9 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -1402,9 +1402,14 @@ args is a list of arguments to notmuch.  ex: (\"tag\" \"+list\"
 
 Calls to notmuch are queued and called asynchronously."
   (setq notmuch-asynch-queue (append notmuch-asynch-queue (list args)))
-  (when (= (length notmuch-asynch-queue) 1)
+  (when (and (= (length notmuch-asynch-queue) 1)
+	     (not (get-process "notmuch-process")))
     (apply 'notmuch-call-notmuch-process-asynch (pop notmuch-asynch-queue))))
-  
+
+(defun notmuch-asynch-sleep-sentinel (process event)
+  "After we sleep, try a command from the notmuch queue"
+  (apply 'notmuch-call-notmuch-process-asynch (pop notmuch-asynch-queue)))
+
 (defun notmuch-call-notmuch-process-asynch-sentinel (process event)
   "Handle the exit of a notmuch asynch process.
 
@@ -1416,11 +1421,16 @@ command, try it again."
     (if (= (process-exit-status process) 0)
 	(kill-buffer (buffer-name (process-buffer process)))
 	(if (search-forward "Unable to acquire database write lock" nil t)
-	    (apply 'notmuch-call-notmuch-process-asynch (cdr (process-command process)))
+	    (progn
+	      (push (cdr (process-command process)) notmuch-asynch-queue)
+	      (set-process-sentinel 
+	       (start-process "notmuch-sleep" nil "sleep" "3")
+	       'notmuch-asynch-sleep-sentinel))
 	    (error (format "%s: %s" (join-string-list (process-command process))
 			   (buffer-string))))))
   (apply 'notmuch-call-notmuch-process-asynch (pop notmuch-asynch-queue)))
 
+
 (defun notmuch-call-notmuch-process (&rest args)
   "Synchronously invoke \"notmuch\" with the given list of arguments.
 
-- 
1.6.3.3

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[Daniel Schoepe]

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

On Tue, 23 Feb 2010 11:32:51 -0500, James Vasile <james@hackervisions.org> wrote:
> Added notmuch-enqueue-asynch to replace calls to
> notmuch-call-notmuch-process.  Calls to notmuch are then queued and
> executed asynchronously.  If the db is busy and we get an error saying
> it was locked, keep trying until the db is no longer busy.  Errors go
> in a buffer as per usual.

I discovered this patch a while ago and it almost applies cleanly (the
conflicts are easy to fix though) and it has made the emacs UI for
notmuch _much_ more responsive and enjoyable for me. As discussed on
IRC, issues such as where to handle retries in case the database is
locked should probably be handled in the notmuch binary instead of each
UI.

I am however unaware of any patches to accomplish that and with the pace
of development / patch reviews the way it is at the moment, this patch
is a really helpful stop-gap (performing calls to notmuch asynchronously
is useful independently of where concurrency issues are handled though)
for me. If someone else agrees, I can fix the conflicts for this patch
and send the revised version to the list.

Cheers,
Daniel

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

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[David Edmondson]

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

On Fri, 21 Oct 2011 22:50:48 +0200, Daniel Schoepe <daniel@schoepe.org> wrote:
> On Tue, 23 Feb 2010 11:32:51 -0500, James Vasile <james@hackervisions.org> wrote:
> > Added notmuch-enqueue-asynch to replace calls to
> > notmuch-call-notmuch-process.  Calls to notmuch are then queued and
> > executed asynchronously.  If the db is busy and we get an error saying
> > it was locked, keep trying until the db is no longer busy.  Errors go
> > in a buffer as per usual.
> 
> I discovered this patch a while ago and it almost applies cleanly (the
> conflicts are easy to fix though) and it has made the emacs UI for
> notmuch _much_ more responsive and enjoyable for me. As discussed on
> IRC, issues such as where to handle retries in case the database is
> locked should probably be handled in the notmuch binary instead of each
> UI.
>
> I am however unaware of any patches to accomplish that and with the pace
> of development / patch reviews the way it is at the moment, this patch
> is a really helpful stop-gap (performing calls to notmuch asynchronously
> is useful independently of where concurrency issues are handled though)
> for me. If someone else agrees, I can fix the conflicts for this patch
> and send the revised version to the list.

It would be good to have an updated version of this patch if it is still
considered useful, though I don't suffer particularly from lag in the
emacs UI.

One concern is the string searching on the output of 'notmuch' to detect
the database locked condition. Perhaps 'notmuch' could be made to
return a specific error code in that case, negating the need to string
match?

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

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[Aaron Ecay]

On Fri, 30 Dec 2011 10:52:17 +0000, David Edmondson <dme@dme.org> wrote:
> > I discovered this patch a while ago and it almost applies cleanly (the
> > conflicts are easy to fix though) and it has made the emacs UI for
> > notmuch _much_ more responsive and enjoyable for me.

This is definitely true for me as well.  So much so that I reinvented
this patch from scratch a couple of weeks ago.  (As Picasso said, good
artists copy, great artists steal.)

> > As discussed on IRC, issues such as where to handle retries in case
> > the database is locked should probably be handled in the notmuch
> > binary instead of each UI.

Hmm.  When my implementation detects that it cannot get the DB lock
after trying several times (up to ~2 mins of waiting), it stops
trying but continues to queue up operations.  The user can then
manually restart the queue.  This isn’t a principled solution, but
it avoids losing tagging operations from emacs while a long-running
process has the DB locked.  (My original motivation for writing the
patch was actually that my new-mail script sometimes holds the DB
lock when I am trying to read mail, leading to emacs errors and
dropped tagging operations.  The speedup was a welcome side effect.)

I haven’t had a chance to carefully look at the patch you found, so IDK
what its behavior would be in this case.

(The other thing that I dislike about the patch you found is that it
uses a call to the “sleep” command to wait, rather than using built-in
emacs functionality.)

> It would be good to have an updated version of this patch if it is still
> considered useful, though I don't suffer particularly from lag in the
> emacs UI.

The emacs UI has small delays, which dropped away when I applied my
version of this patch.  After being conditioned to expect them, I was
surprised to find how much faster everything seemed.  My hardware is old
but not ancient (5.5 year old Macbook; I suspect the bottleneck for
notmuch is the 5400rpm HDD).  Depending on your setup, you too might be
pleasantly surprised.

The test suite is borked on OS X and I don’t have access to a linux
machine while traveling.  I was planning on sending my patch in early
Jan. when I had a chance to verify it under the test suite (probably
requiring some changes to the emacs test library to make it async-safe).
I also haven’t tested the patch on Emacs versions older than 24 – but I
don’t think there are any impediments to compatibility with v.23 (not
sure about earlier versions).

Since there is interest, I’ll go ahead and send it now with all the
usual caveats about code under development.  I have been using the patch
for a couple of weeks without problems, though.  Daniel, if you want to
un-conflict (and squash) the patches from James that might be useful, at
least to compare the two approaches.

Aaron

-----cut-here-----

From f0a0fe04254d9b5e17c873b293c6a5a270cb909a Mon Sep 17 00:00:00 2001
From: Aaron Ecay <aaronecay@gmail.com>
Date: Mon, 19 Dec 2011 12:14:31 -0500
Subject: [PATCH] [emacs] add async tagging support

still a WIP
---
 emacs/notmuch.el |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fde2377..ca077c2 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -446,6 +446,59 @@ Complete list of currently available key bindings:
   (let ((message-id (notmuch-search-find-thread-id)))
     (notmuch-mua-new-reply message-id prompt-for-sender)))
 
+(defvar notmuch-process-queue nil
+  "Queue of pending notmuch tag operations.
+
+Each entry in the queue is a list of strings, which are arguments
+to be passed to the notmuch command.")
+
+(defvar notmuch-process-current-command nil
+  "The currently executing notmuch command arguments")
+
+(defvar notmuch-process-wait-time 1.0
+  "How long to wait for the db lock, in seconds.")
+
+(defvar notmuch-process-error nil)
+
+(defun notmuch-process-restart-queue ()
+  (interactive)
+  (setq notmuch-process-error nil)
+  (notmuch-process-kick-queue))
+
+(defun notmuch-process-kick-queue ()
+  (when notmuch-process-error
+    ;; TODO: better error msg, tell how to manually kick
+    (error "Notmuch couldn't get the DB lock after trying for more than a minute."))
+  (when (and notmuch-process-queue
+	     (memq (process-status "notmuch") '(exit nil)))
+    (let ((args (car notmuch-process-queue))
+	  proc)
+      (setq notmuch-process-queue (cdr notmuch-process-queue)
+	    notmuch-process-current-command args
+	    proc (apply #'start-process "notmuch"
+			(get-buffer-create "*Notmuch output*")
+			notmuch-command args))
+      (set-process-sentinel proc #'notmuch-process-sentinel))))
+
+(defun notmuch-process-sentinel (proc status)
+  (if (equal status "finished\n")
+      (progn
+	(setq notmuch-process-wait-time 1.0
+	      notmuch-process-current-command nil)
+	(notmuch-process-kick-queue))
+    (setq notmuch-process-wait-time (* 2 notmuch-process-wait-time)
+	  notmuch-process-queue (cons notmuch-process-current-command
+				      notmuch-process-queue))
+    (if (<= notmuch-process-wait-time 64)
+	(progn
+	  (run-at-time notmuch-process-wait-time nil
+		       #'notmuch-process-kick-queue))
+      (setq notmuch-process-error t))))
+
+(defun notmuch-process-queue-command (&rest args)
+  (setq notmuch-process-queue (append notmuch-process-queue (list args)))
+  (notmuch-process-kick-queue))
+
 (defun notmuch-call-notmuch-process (&rest args)
   "Synchronously invoke \"notmuch\" with the given list of arguments.
 
@@ -474,7 +527,7 @@ messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
 directly, so that hooks specified in notmuch-before-tag-hook and
 notmuch-after-tag-hook will be run."
   (run-hooks 'notmuch-before-tag-hook)
-  (apply 'notmuch-call-notmuch-process
+  (apply 'notmuch-process-queue-command
 	 (append (list "tag") tags (list "--" query)))
   (run-hooks 'notmuch-after-tag-hook))
 
-- 
1.7.8.2

-----cut-here-----

-- 
Aaron Ecay

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[David Edmondson]

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

At a quick look your code seems fine. I wonder if async operation should
be optional (and non-default?) given that it has different failure
modes?

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

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[Ethan Glasser-Camp]

James Vasile <james@hackervisions.org> writes:

> Added notmuch-enqueue-asynch to replace calls to
> notmuch-call-notmuch-process.  Calls to notmuch are then queued and
> executed asynchronously.  If the db is busy and we get an error saying
> it was locked, keep trying until the db is no longer busy.  Errors go
> in a buffer as per usual.

Hi! I decided to review some of the outstanding patches on the nmbug
queue.

This one doesn't apply cleanly. First, notmuch.el is now in
emacs/. Secondly, tagging happens only one place in notmuch-tag.el.

The follow-up patch doesn't apply either, and neither does the WIP patch
posted later in the thread by Aaron Ecay.

It seems like this patch series is useful for some people, but there are
some design issues yet to be worked out.

I propose that this thread be tagged notmuch::stale.

Ethan

Re: [PATCH] Calls to notmuch get queued and executed asynchronously.

[David Bremner]

Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:

> It seems like this patch series is useful for some people, but there are
> some design issues yet to be worked out.
>
> I propose that this thread be tagged notmuch::stale.

Go for it, now that you have awesome power to tag things in nmbug.

A periodic reminder, anyone who wants werte access to the nmbug tag
database should send me a ssh public key.

David