[PATCH 1/2] Close message file after parsing message headers

classic Classic list List threaded Threaded
5 messages Options
Mikhail Gusarov Mikhail Gusarov
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Close message file after parsing message headers

Keeping unused files open helps to see "Too many open files" often.

Signed-off-by: Mikhail Gusarov <[hidden email]>
---
 lib/message-file.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/lib/message-file.c b/lib/message-file.c
index 8a3f8ee..197ab01 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -325,6 +325,11 @@ notmuch_message_file_get_header (notmuch_message_file_t *message,
     return decoded_value;
     }
 
+    if (message->parsing_finished) {
+        fclose (message->file);
+        message->file = NULL;
+    }
+
     if (message->line)
  free (message->line);
     message->line = NULL;
--
1.6.3.3


Mikhail Gusarov Mikhail Gusarov
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Include <stdint.h> to get uint32_t in C++ file with gcc 4.4


Signed-off-by: Mikhail Gusarov <[hidden email]>
---
 lib/message.cc |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 72c350f..a4b090b 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -21,6 +21,8 @@
 #include "notmuch-private.h"
 #include "database-private.h"
 
+#include <stdint.h>
+
 #include <gmime/gmime.h>
 
 #include <xapian.h>
--
1.6.3.3


Carl Worth-2 Carl Worth-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Close message file after parsing message headers

In reply to this post by Mikhail Gusarov
On Tue, 17 Nov 2009 21:28:37 +0600, Mikhail Gusarov <[hidden email]> wrote:
> Keeping unused files open helps to see "Too many open files" often.
>
> Signed-off-by: Mikhail Gusarov <[hidden email]>
...
On Tue, 17 Nov 2009 21:28:38 +0600, Mikhail Gusarov <[hidden email]> wrote:
>
> Signed-off-by: Mikhail Gusarov <[hidden email]>
> ---
>  lib/message.cc |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Hi Mikhail,

Welcome to notmuch, and thanks for these patches! I've pushed both of
them out now.

Keith ran into the same problem of "too many open files" and wrote a
more complex fix, (which included what you did here). His code can be
seen at:

        git://keithp.com/git/notmuch

I didn't apply Keith's fix yet, because I think I'd rather just fix the
indexer to store the In-Reply-To header in a separate term prefix from
the term used for the References header[*]. That will then let us lookup
the in-reply-to value later for thread constructions without having to
open the original email file at all.

-Carl

[*] Yes, this is my first post to our new mailing list and I'm already
spouting off about "terms" and "prefixes" without any definitions. I
apologize for that. I hope that people will ask questions freely here on
the list whenever anything is not clear, and I'll be glad to explain
things as needed. (Then when can shove answers into a HACKING document.)

PS. This reply is a great example of a feature that notmuch *almost*
supports already---repling to multiple messages at once. The "notmuch
reply" command line does everything necessary to make this work, but we
haven't yet hooked up any keybindings for this in the emacs client yet.
Obviously, 'r' from the search view could reply to the entire thread.
But when viewing a thread, anyone have a good keybinding suggestion?
(There's obviously 'R' as opposed to 'r', but I think we'll probably
want to distinguish "reply to sender" from "reply to all" before trying
to distinguish "reply to message" from "reply to thread" (which I
imagine is more rare of an operation).

Keith Packard Keith Packard
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Close message file after parsing message headers

On Tue, 17 Nov 2009 09:13:27 -0800, Carl Worth <[hidden email]> wrote:

> I didn't apply Keith's fix yet, because I think I'd rather just fix the
> indexer to store the In-Reply-To header in a separate term prefix from
> the term used for the References header[*]. That will then let us lookup
> the in-reply-to value later for thread constructions without having to
> open the original email file at all.

Threading the message also involves displaying the from and to contents,
which requires opening the message file. The alternative to the fix I
provided is to just parse all of the message headers when first opening
the message; it could then be immediately closed and the hash referred
to for all header data. Given the choice, just having the caller say
when it has finished with a message is probably a reasonable option...

-keith

Carl Worth-2 Carl Worth-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Close message file after parsing message headers

On Tue, 17 Nov 2009 13:15:25 -0800, Keith Packard <[hidden email]> wrote:
> Threading the message also involves displaying the from and to contents,
> which requires opening the message file. The alternative to the fix I
> provided is to just parse all of the message headers when first opening
> the message; it could then be immediately closed and the hash referred
> to for all header data. Given the choice, just having the caller say
> when it has finished with a message is probably a reasonable option...

Hi Keith,

Once I finally got back on the ground again, I pushed out a revised
version of your patch, (didn't need the reply-to stuff anymore since I
had fixed that differently in the meantime).

I'm pretty happy with the state of this portion of the code now.

Thanks Keith and Mikhail for your input on and code to fix this bug.

-Carl