ae/2a6efb1c04958932842d17b92cc0bae5ee38b6

   1 Return-Path: <cworth@cworth.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id B327C431FBD\r
   6         for <notmuch@notmuchmail.org>; Mon,  8 Feb 2010 11:42:05 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -3.186\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-3.186 tagged_above=-999 required=5\r
  12         tests=[ALL_TRUSTED=-1.8, AWL=1.213, BAYES_00=-2.599] autolearn=ham\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id Jhn3Z9BWJoFr for <notmuch@notmuchmail.org>;\r
  16         Mon,  8 Feb 2010 11:42:04 -0800 (PST)\r
  17 Received: from yoom.home.cworth.org (localhost [127.0.0.1])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 89866431FBC\r
  19         for <notmuch@notmuchmail.org>; Mon,  8 Feb 2010 11:42:04 -0800 (PST)\r
  20 Received: by yoom.home.cworth.org (Postfix, from userid 1000)\r
  21         id 1BF6155011E; Mon,  8 Feb 2010 11:42:04 -0800 (PST)\r
  22 From: Carl Worth <cworth@cworth.org>\r
  23 To: notmuch@notmuchmail.org\r
  24 Date: Mon, 08 Feb 2010 11:42:04 -0800\r
  25 Message-ID: <87mxzjpl9v.fsf@yoom.home.cworth.org>\r
  26 MIME-Version: 1.0\r
  27 Content-Type: text/plain; charset=us-ascii\r
  28 Subject: [notmuch] [PATCH] Switch from random to sequential thread\r
  29         identifiers.\r
  30 X-BeenThere: notmuch@notmuchmail.org\r
  31 X-Mailman-Version: 2.1.13\r
  32 Precedence: list\r
  33 List-Id: "Use and development of the notmuch mail system."\r
  34         <notmuch.notmuchmail.org>\r
  35 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  36         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  37 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  38 List-Post: <mailto:notmuch@notmuchmail.org>\r
  39 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  40 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  41         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  42 X-List-Received-Date: Mon, 08 Feb 2010 19:42:05 -0000\r
  43 \r
  44 The sequential identifiers have the advantage of being guaranteed to\r
  45 be unique (until we overflow a 64-bit unsigned integer), and also take\r
  46 up slightly less space in the "notmuch search" output (20 columns\r
  47 rather than 32).\r
  48 \r
  49 This change also has the side effect of fixing a bug where notmuch\r
  50 could block on /dev/random at startup (waiting for some entropy to\r
  51 appear). This bug was hit hard by the test suite, (which could easily\r
  52 exhaust the available entropy on common systems---resulting in large\r
  53 delays of the test suite).\r
  54 \r
  55 ---\r
  56 \r
  57 I'm sending this patch to the mailing-list rather than pushing it\r
  58 directly so that any authors of user interfaces can ensure that they are\r
  59 ready for the length of thread identifiers in "notmuch search" output to\r
  60 change.\r
  61 \r
  62 I tested that the emacs client doesn't need any change in this regard.\r
  63 \r
  64 And the change doesn't introduce any compatibility with an existing\r
  65 database, so no rebuild required (hurrah!).\r
  66 \r
  67 -Carl\r
  68 \r
  69  lib/database-private.h |    7 +++++-\r
  70  lib/database.cc        |   52 ++++++++++++++++++++++++++++++++++++++++++++---\r
  71  lib/message.cc         |   46 ------------------------------------------\r
  72  test/notmuch-test      |    2 +-\r
  73  4 files changed, 55 insertions(+), 52 deletions(-)\r
  74 \r
  75 diff --git a/lib/database-private.h b/lib/database-private.h\r
  76 index 5891584..5bb6e86 100644\r
  77 --- a/lib/database-private.h\r
  78 +++ b/lib/database-private.h\r
  79 @@ -27,14 +27,19 @@\r
  80  \r
  81  struct _notmuch_database {\r
  82      notmuch_bool_t exception_reported;\r
  83 +\r
  84      char *path;\r
  85 +\r
  86 +    notmuch_bool_t needs_upgrade;\r
  87      notmuch_database_mode_t mode;\r
  88      Xapian::Database *xapian_db;\r
  89 +\r
  90 +    uint64_t last_thread_id;\r
  91 +\r
  92      Xapian::QueryParser *query_parser;\r
  93      Xapian::TermGenerator *term_gen;\r
  94      Xapian::ValueRangeProcessor *value_range_processor;\r
  95  \r
  96 -    notmuch_bool_t needs_upgrade;\r
  97  };\r
  98  \r
  99  /* Convert tags from Xapian internal format to notmuch format.\r
 100 diff --git a/lib/database.cc b/lib/database.cc\r
 101 index cce7847..65ff12e 100644\r
 102 --- a/lib/database.cc\r
 103 +++ b/lib/database.cc\r
 104 @@ -533,6 +533,8 @@ notmuch_database_open (const char *path,\r
 105      notmuch->needs_upgrade = FALSE;\r
 106      notmuch->mode = mode;\r
 107      try {\r
 108 +       string last_thread_id;\r
 109 +\r
 110         if (mode == NOTMUCH_DATABASE_MODE_READ_WRITE) {\r
 111             notmuch->xapian_db = new Xapian::WritableDatabase (xapian_path,\r
 112                                                                Xapian::DB_CREATE_OR_OPEN);\r
 113 @@ -567,6 +569,20 @@ notmuch_database_open (const char *path,\r
 114                          notmuch_path, version, NOTMUCH_DATABASE_VERSION);\r
 115             }\r
 116         }\r
 117 +\r
 118 +       last_thread_id = notmuch->xapian_db->get_metadata ("last_thread_id");\r
 119 +       if (last_thread_id.empty ()) {\r
 120 +           notmuch->last_thread_id = 0;\r
 121 +       } else {\r
 122 +           const char *str;\r
 123 +           char *end;\r
 124 +\r
 125 +           str = last_thread_id.c_str ();\r
 126 +           notmuch->last_thread_id = strtoull (str, &end, 10);\r
 127 +           if (*end != '\0')\r
 128 +               INTERNAL_ERROR ("Malformed database last_thread_id: %s", str);\r
 129 +       }\r
 130 +\r
 131         notmuch->query_parser = new Xapian::QueryParser;\r
 132         notmuch->term_gen = new Xapian::TermGenerator;\r
 133         notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));\r
 134 @@ -1278,14 +1294,38 @@ _notmuch_database_link_message_to_children (notmuch_database_t *notmuch,\r
 135      return ret;\r
 136  }\r
 137  \r
 138 +static const char *\r
 139 +_notmuch_database_generate_thread_id (notmuch_database_t *notmuch)\r
 140 +{\r
 141 +    /* 20 bytes (+ terminator) for ASCII decimal representation of\r
 142 +     * a 64-bit integer. */\r
 143 +    static char thread_id[21];\r
 144 +    Xapian::WritableDatabase *db;\r
 145 +\r
 146 +    db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);\r
 147 +\r
 148 +    notmuch->last_thread_id++;\r
 149 +\r
 150 +    sprintf (thread_id, "%020llu", notmuch->last_thread_id);\r
 151 +\r
 152 +    db->set_metadata ("last_thread_id", thread_id);\r
 153 +\r
 154 +    return thread_id;\r
 155 +}\r
 156 +\r
 157  /* Given a (mostly empty) 'message' and its corresponding\r
 158   * 'message_file' link it to existing threads in the database.\r
 159   *\r
 160   * We first look at 'message_file' and its link-relevant headers\r
 161   * (References and In-Reply-To) for message IDs. We also look in the\r
 162 - * database for existing message that reference 'message'.\r
 163 + * database for existing message that reference 'message'. In either\r
 164 + * case, we will assign to the current message the first thread_id\r
 165 + * found (through either parent or child). We will also merge any\r
 166 + * existing, distinct threads where this message belongs to both,\r
 167 + * (which is not uncommon when mesages are processed out of order).\r
 168   *\r
 169 - * The end result is to call _notmuch_message_ensure_thread_id which\r
 170 + * Finally, if not thread ID has been found through parent or child,\r
 171 + * we call _notmuch_message_generate_thread_id to generate a new\r
 172   * generates a new thread ID if the message doesn't connect to any\r
 173   * existing threads.\r
 174   */\r
 175 @@ -1308,8 +1348,12 @@ _notmuch_database_link_message (notmuch_database_t *notmuch,\r
 176      if (status)\r
 177         return status;\r
 178  \r
 179 -    if (thread_id == NULL)\r
 180 -       _notmuch_message_ensure_thread_id (message);\r
 181 +    /* If not part of any existing thread, generate a new thread ID. */\r
 182 +    if (thread_id == NULL) {\r
 183 +       thread_id = _notmuch_database_generate_thread_id (notmuch);\r
 184 +\r
 185 +       _notmuch_message_add_term (message, "thread", thread_id);\r
 186 +    }\r
 187  \r
 188      return NOTMUCH_STATUS_SUCCESS;\r
 189  }\r
 190 diff --git a/lib/message.cc b/lib/message.cc\r
 191 index f0e905b..0195050 100644\r
 192 --- a/lib/message.cc\r
 193 +++ b/lib/message.cc\r
 194 @@ -42,13 +42,6 @@ struct _notmuch_message {\r
 195      Xapian::Document doc;\r
 196  };\r
 197  \r
 198 -/* "128 bits of thread-id ought to be enough for anybody" */\r
 199 -#define NOTMUCH_THREAD_ID_BITS  128\r
 200 -#define NOTMUCH_THREAD_ID_DIGITS (NOTMUCH_THREAD_ID_BITS / 4)\r
 201 -typedef struct _thread_id {\r
 202 -    char str[NOTMUCH_THREAD_ID_DIGITS + 1];\r
 203 -} thread_id_t;\r
 204 -\r
 205  /* We end up having to call the destructor explicitly because we had\r
 206   * to use "placement new" in order to initialize C++ objects within a\r
 207   * block that we allocated with talloc. So C++ is making talloc\r
 208 @@ -557,45 +550,6 @@ _notmuch_message_set_date (notmuch_message_t *message,\r
 209                             Xapian::sortable_serialise (time_value));\r
 210  }\r
 211  \r
 212 -static void\r
 213 -thread_id_generate (thread_id_t *thread_id)\r
 214 -{\r
 215 -    static int seeded = 0;\r
 216 -    FILE *dev_random;\r
 217 -    uint32_t value;\r
 218 -    char *s;\r
 219 -    int i;\r
 220 -\r
 221 -    if (! seeded) {\r
 222 -       dev_random = fopen ("/dev/random", "r");\r
 223 -       if (dev_random == NULL) {\r
 224 -           srand (time (NULL));\r
 225 -       } else {\r
 226 -           fread ((void *) &value, sizeof (value), 1, dev_random);\r
 227 -           srand (value);\r
 228 -           fclose (dev_random);\r
 229 -       }\r
 230 -       seeded = 1;\r
 231 -    }\r
 232 -\r
 233 -    s = thread_id->str;\r
 234 -    for (i = 0; i < NOTMUCH_THREAD_ID_DIGITS; i += 8) {\r
 235 -       value = rand ();\r
 236 -       sprintf (s, "%08x", value);\r
 237 -       s += 8;\r
 238 -    }\r
 239 -}\r
 240 -\r
 241 -void\r
 242 -_notmuch_message_ensure_thread_id (notmuch_message_t *message)\r
 243 -{\r
 244 -    /* If not part of any existing thread, generate a new thread_id. */\r
 245 -    thread_id_t thread_id;\r
 246 -\r
 247 -    thread_id_generate (&thread_id);\r
 248 -    _notmuch_message_add_term (message, "thread", thread_id.str);\r
 249 -}\r
 250 -\r
 251  /* Synchronize changes made to message->doc out into the database. */\r
 252  void\r
 253  _notmuch_message_sync (notmuch_message_t *message)\r
 254 diff --git a/test/notmuch-test b/test/notmuch-test\r
 255 index 2e5eb24..cac5705 100755\r
 256 --- a/test/notmuch-test\r
 257 +++ b/test/notmuch-test\r
 258 @@ -146,7 +146,7 @@ add_message ()\r
 259  }\r
 260  \r
 261  NOTMUCH_IGNORED_OUTPUT_REGEXP='^Processed [0-9]*( total)? file|Found [0-9]* total file'\r
 262 -NOTMUCH_THREAD_ID_SQUELCH='s/thread:................................/thread:XXX/'\r
 263 +NOTMUCH_THREAD_ID_SQUELCH='s/thread:..................../thread:XXX/'\r
 264  execute_expecting ()\r
 265  {\r
 266      args=$1\r
 267 -- \r
 268 1.6.5.7\r
 269 \r
 270 \r