70/b99f317172786ddca5164f41ef7ff0a76333ad

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