Re: [notmuch] [PATCH] Store documents for message-ids we haven't seen
On Sun, 20 Dec 2009 20:27:32 +, James Westby
wrote:
> One case that isn't handled is when we don't find the thread
> id of the parent, but then find the message itself. I believe
> this case shouldn't happen, but you never know.
It really shouldn't happen since we are holding a write lock on the
database, (so there's no possible race condition here with another
client delivering the parent message).
But since you almost can't help but detect the case, (just noticing a
NOTMUCH_STATUS_SUCCESS value from _create_for_message_id), please put an
INTERNAL_ERROR there rather than marching along with an incorrect thread
ID.
> + // We have yet to see the referenced message, generate a thread id
> + // for it if needed and store a dummy message for the parent. If we
> + // find the mail later we will replace the dummy.
Call me old-fashioned if you will, but I'd much rather have C style
multi-line comments (/* ... */) rather than these C++-style comments
with //.
> + if (private_status ==
> NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
> + // expected
And I think this comment deserves a complete sentence before the
condition. Something like:
/* We expect this call to create a new document (return NO_DOCUMENT_FOUND) */
-Carl
pgpC7E8uY0WI7.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[notmuch] [PATCH] Store documents for message-ids we haven't seen
On Sun, 20 Dec 2009 20:27:32 +, James Westby
wrote:
> One case that isn't handled is when we don't find the thread
> id of the parent, but then find the message itself. I believe
> this case shouldn't happen, but you never know.
It really shouldn't happen since we are holding a write lock on the
database, (so there's no possible race condition here with another
client delivering the parent message).
But since you almost can't help but detect the case, (just noticing a
NOTMUCH_STATUS_SUCCESS value from _create_for_message_id), please put an
INTERNAL_ERROR there rather than marching along with an incorrect thread
ID.
> + // We have yet to see the referenced message, generate a thread id
> + // for it if needed and store a dummy message for the parent. If we
> + // find the mail later we will replace the dummy.
Call me old-fashioned if you will, but I'd much rather have C style
multi-line comments (/* ... */) rather than these C++-style comments
with //.
> + if (private_status ==
> NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
> + // expected
And I think this comment deserves a complete sentence before the
condition. Something like:
/* We expect this call to create a new document (return NO_DOCUMENT_FOUND) */
-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL:
[notmuch] [PATCH] Store documents for message-ids we haven't seen
When scanning the maildir there can be earlier messages
in the thread that we haven't seen, either due to mail
delays, or because they weren't sent. This can break the
threading.
Therefore we store stub documents for every message id
we see, so that we can link them in to the threads. If
we see the message later then we will replace the document
with the full information.
---
Here's the third part of the patch, that actually stores
the stubs and links the threads. It's fairly simple,
though we may want to store a little more in the stubs.
One case that isn't handled is when we don't find the thread
id of the parent, but then find the message itself. I believe
this case shouldn't happen, but you never know.
lib/database.cc | 25 +
lib/message.cc|9 +
lib/notmuch-private.h | 10 ++
3 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 64f29b9..6ee8068 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -782,8 +782,33 @@ _notmuch_database_link_message_to_parents
(notmuch_database_t *notmuch,
parent_message_id);
if (parent_thread_id == NULL) {
+ notmuch_private_status_t private_status;
+ notmuch_message_t *new_parent_message;
+ thread_id_t parent_thread_id_t;
+
+ // We have yet to see the referenced message, generate a thread id
+ // for it if needed and store a dummy message for the parent. If we
+ // find the mail later we will replace the dummy.
_notmuch_message_add_term (message, "reference",
parent_message_id);
+ if (*thread_id == NULL) {
+ thread_id_generate (&parent_thread_id_t);
+ *thread_id = parent_thread_id_t.str;
+ }
+ new_parent_message = _notmuch_message_create_for_message_id
(notmuch,
+ parent_message_id, &private_status);
+ if (private_status) {
+ if (private_status ==
NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+ // expected
+ _notmuch_message_add_term (new_parent_message,
"thread", *thread_id);
+ _notmuch_message_sync (new_parent_message);
+ } else {
+ ret = COERCE_STATUS (private_status,
+"Cannot create parent message");
+ goto DONE;
+ }
+ }
+ _notmuch_message_add_term (message, "thread", *thread_id);
} else {
if (*thread_id == NULL) {
*thread_id = talloc_strdup (message, parent_thread_id);
diff --git a/lib/message.cc b/lib/message.cc
index 7129d59..dff02c4 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -42,13 +42,6 @@ struct _notmuch_message {
Xapian::Document doc;
};
-/* "128 bits of thread-id ought to be enough for anybody" */
-#define NOTMUCH_THREAD_ID_BITS 128
-#define NOTMUCH_THREAD_ID_DIGITS (NOTMUCH_THREAD_ID_BITS / 4)
-typedef struct _thread_id {
-char str[NOTMUCH_THREAD_ID_DIGITS + 1];
-} thread_id_t;
-
/* We end up having to call the destructor explicitly because we had
* to use "placement new" in order to initialize C++ objects within a
* block that we allocated with talloc. So C++ is making talloc
@@ -551,7 +544,7 @@ _notmuch_message_set_date (notmuch_message_t *message,
Xapian::sortable_serialise (time_value));
}
-static void
+void
thread_id_generate (thread_id_t *thread_id)
{
static int seeded = 0;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index cf65fd9..cf75b4a 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -141,6 +141,13 @@ typedef enum _notmuch_private_status {
: \
(notmuch_status_t) private_status)
+/* "128 bits of thread-id ought to be enough for anybody" */
+#define NOTMUCH_THREAD_ID_BITS 128
+#define NOTMUCH_THREAD_ID_DIGITS (NOTMUCH_THREAD_ID_BITS / 4)
+typedef struct _thread_id {
+char str[NOTMUCH_THREAD_ID_DIGITS + 1];
+} thread_id_t;
+
/* database.cc */
/* Lookup a prefix value by name.
@@ -347,6 +354,9 @@ void
_notmuch_message_add_reply (notmuch_message_t *message,
notmuch_message_node_t *reply);
+void
+thread_id_generate (thread_id_t *thread_id);
+
/* sha1.c */
char *
--
1.6.3.3
[notmuch] [PATCH] Store documents for message-ids we haven't seen
When scanning the maildir there can be earlier messages
in the thread that we haven't seen, either due to mail
delays, or because they weren't sent. This can break the
threading.
Therefore we store stub documents for every message id
we see, so that we can link them in to the threads. If
we see the message later then we will replace the document
with the full information.
---
Here's the third part of the patch, that actually stores
the stubs and links the threads. It's fairly simple,
though we may want to store a little more in the stubs.
One case that isn't handled is when we don't find the thread
id of the parent, but then find the message itself. I believe
this case shouldn't happen, but you never know.
lib/database.cc | 25 +
lib/message.cc|9 +
lib/notmuch-private.h | 10 ++
3 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 64f29b9..6ee8068 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -782,8 +782,33 @@ _notmuch_database_link_message_to_parents
(notmuch_database_t *notmuch,
parent_message_id);
if (parent_thread_id == NULL) {
+ notmuch_private_status_t private_status;
+ notmuch_message_t *new_parent_message;
+ thread_id_t parent_thread_id_t;
+
+ // We have yet to see the referenced message, generate a thread id
+ // for it if needed and store a dummy message for the parent. If we
+ // find the mail later we will replace the dummy.
_notmuch_message_add_term (message, "reference",
parent_message_id);
+ if (*thread_id == NULL) {
+ thread_id_generate (&parent_thread_id_t);
+ *thread_id = parent_thread_id_t.str;
+ }
+ new_parent_message = _notmuch_message_create_for_message_id
(notmuch,
+ parent_message_id, &private_status);
+ if (private_status) {
+ if (private_status ==
NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+ // expected
+ _notmuch_message_add_term (new_parent_message,
"thread", *thread_id);
+ _notmuch_message_sync (new_parent_message);
+ } else {
+ ret = COERCE_STATUS (private_status,
+"Cannot create parent message");
+ goto DONE;
+ }
+ }
+ _notmuch_message_add_term (message, "thread", *thread_id);
} else {
if (*thread_id == NULL) {
*thread_id = talloc_strdup (message, parent_thread_id);
diff --git a/lib/message.cc b/lib/message.cc
index 7129d59..dff02c4 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -42,13 +42,6 @@ struct _notmuch_message {
Xapian::Document doc;
};
-/* "128 bits of thread-id ought to be enough for anybody" */
-#define NOTMUCH_THREAD_ID_BITS 128
-#define NOTMUCH_THREAD_ID_DIGITS (NOTMUCH_THREAD_ID_BITS / 4)
-typedef struct _thread_id {
-char str[NOTMUCH_THREAD_ID_DIGITS + 1];
-} thread_id_t;
-
/* We end up having to call the destructor explicitly because we had
* to use "placement new" in order to initialize C++ objects within a
* block that we allocated with talloc. So C++ is making talloc
@@ -551,7 +544,7 @@ _notmuch_message_set_date (notmuch_message_t *message,
Xapian::sortable_serialise (time_value));
}
-static void
+void
thread_id_generate (thread_id_t *thread_id)
{
static int seeded = 0;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index cf65fd9..cf75b4a 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -141,6 +141,13 @@ typedef enum _notmuch_private_status {
: \
(notmuch_status_t) private_status)
+/* "128 bits of thread-id ought to be enough for anybody" */
+#define NOTMUCH_THREAD_ID_BITS 128
+#define NOTMUCH_THREAD_ID_DIGITS (NOTMUCH_THREAD_ID_BITS / 4)
+typedef struct _thread_id {
+char str[NOTMUCH_THREAD_ID_DIGITS + 1];
+} thread_id_t;
+
/* database.cc */
/* Lookup a prefix value by name.
@@ -347,6 +354,9 @@ void
_notmuch_message_add_reply (notmuch_message_t *message,
notmuch_message_node_t *reply);
+void
+thread_id_generate (thread_id_t *thread_id);
+
/* sha1.c */
char *
--
1.6.3.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
