unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] Calls to notmuch get queued and executed asynchronously.
@ 2010-02-23 16:32 James Vasile
  2010-02-24 19:28 ` James Vasile
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: James Vasile @ 2010-02-23 16:32 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2010-02-23 16:32 [PATCH] Calls to notmuch get queued and executed asynchronously James Vasile
@ 2010-02-24 19:28 ` James Vasile
  2011-10-21 20:50 ` Daniel Schoepe
  2012-10-12  2:21 ` Ethan Glasser-Camp
  2 siblings, 0 replies; 8+ messages in thread
From: James Vasile @ 2010-02-24 19:28 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2010-02-23 16:32 [PATCH] Calls to notmuch get queued and executed asynchronously James Vasile
  2010-02-24 19:28 ` James Vasile
@ 2011-10-21 20:50 ` Daniel Schoepe
  2011-12-30 10:52   ` David Edmondson
  2012-10-12  2:21 ` Ethan Glasser-Camp
  2 siblings, 1 reply; 8+ messages in thread
From: Daniel Schoepe @ 2011-10-21 20:50 UTC (permalink / raw)
  To: James Vasile, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2011-10-21 20:50 ` Daniel Schoepe
@ 2011-12-30 10:52   ` David Edmondson
  2011-12-30 19:21     ` Aaron Ecay
  0 siblings, 1 reply; 8+ messages in thread
From: David Edmondson @ 2011-12-30 10:52 UTC (permalink / raw)
  To: Daniel Schoepe, James Vasile, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2011-12-30 10:52   ` David Edmondson
@ 2011-12-30 19:21     ` Aaron Ecay
  2012-01-03 10:58       ` David Edmondson
  0 siblings, 1 reply; 8+ messages in thread
From: Aaron Ecay @ 2011-12-30 19:21 UTC (permalink / raw)
  To: David Edmondson, Daniel Schoepe, James Vasile, notmuch

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2011-12-30 19:21     ` Aaron Ecay
@ 2012-01-03 10:58       ` David Edmondson
  0 siblings, 0 replies; 8+ messages in thread
From: David Edmondson @ 2012-01-03 10:58 UTC (permalink / raw)
  To: Aaron Ecay, Daniel Schoepe, James Vasile, notmuch

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2010-02-23 16:32 [PATCH] Calls to notmuch get queued and executed asynchronously James Vasile
  2010-02-24 19:28 ` James Vasile
  2011-10-21 20:50 ` Daniel Schoepe
@ 2012-10-12  2:21 ` Ethan Glasser-Camp
  2012-10-12 16:58   ` David Bremner
  2 siblings, 1 reply; 8+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-12  2:21 UTC (permalink / raw)
  To: James Vasile, notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Calls to notmuch get queued and executed asynchronously.
  2012-10-12  2:21 ` Ethan Glasser-Camp
@ 2012-10-12 16:58   ` David Bremner
  0 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2012-10-12 16:58 UTC (permalink / raw)
  To: Ethan Glasser-Camp, notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-10-12 16:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-23 16:32 [PATCH] Calls to notmuch get queued and executed asynchronously James Vasile
2010-02-24 19:28 ` James Vasile
2011-10-21 20:50 ` Daniel Schoepe
2011-12-30 10:52   ` David Edmondson
2011-12-30 19:21     ` Aaron Ecay
2012-01-03 10:58       ` David Edmondson
2012-10-12  2:21 ` Ethan Glasser-Camp
2012-10-12 16:58   ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).