[PATCH 2/6] lib: Add per-message last modification tracking

[Austin Clements]

Hi David. I haven't had a chance to look back at the original code, but
your follow-up expanded comment agrees with how I remember this code
working.
On Aug 7, 2015 4:41 PM, "David Bremner"  wrote:

> Daniel Schoepe  writes:
>
>
> > On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
> >> +/* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
> >> + * track modification revisions.  Give all messages a
> >> + * revision of 1.
> >> + */
> >> +if (new_features & NOTMUCH_FEATURE_LAST_MOD)
> >> +_notmuch_message_upgrade_last_mod (message);
> >> [..]
> >> +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
> >> + * must call _notmuch_message_sync. */
> >> +void
> >> +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
> >> +{
> >> +/* _notmuch_message_sync will update the last modification
> >> + * revision; we just have to ask it to. */
> >> +message->modified = TRUE;
> >> +}
> >> +
> >
> > The comment in the first part says that message without LAST_MOD get a
> > revision of 1, but as far as I can tell, _notmuch_message_sync will
> > actually put the next revision number on the message. If this is what's
> > happening, either the comment or the behavior should be changed,
> > depending on what's intended.
>
> I think the behaviour is OK, but you're correct the comment is
> wrong. I'll see if Austin has any comment on that. I guess it would be
> harmless to initialize upgraded messages to some low revision number,
> but I don't see the benefit.
>
-- next part --
An HTML attachment was scrubbed...
URL: 

Re: [PATCH 2/6] lib: Add per-message last modification tracking

[Austin Clements]

Hi David. I haven't had a chance to look back at the original code, but
your follow-up expanded comment agrees with how I remember this code
working.
On Aug 7, 2015 4:41 PM, David Bremner da...@tethera.net wrote:

 Daniel Schoepe dan...@schoepe.org writes:


  On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
  +/* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
  + * track modification revisions.  Give all messages a
  + * revision of 1.
  + */
  +if (new_features  NOTMUCH_FEATURE_LAST_MOD)
  +_notmuch_message_upgrade_last_mod (message);
  [..]
  +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
  + * must call _notmuch_message_sync. */
  +void
  +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
  +{
  +/* _notmuch_message_sync will update the last modification
  + * revision; we just have to ask it to. */
  +message-modified = TRUE;
  +}
  +
 
  The comment in the first part says that message without LAST_MOD get a
  revision of 1, but as far as I can tell, _notmuch_message_sync will
  actually put the next revision number on the message. If this is what's
  happening, either the comment or the behavior should be changed,
  depending on what's intended.

 I think the behaviour is OK, but you're correct the comment is
 wrong. I'll see if Austin has any comment on that. I guess it would be
 harmless to initialize upgraded messages to some low revision number,
 but I don't see the benefit.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/6] lib: Add per-message last modification tracking

[David Bremner]

Daniel Schoepe  writes:


> On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
>> +/* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
>> + * track modification revisions.  Give all messages a
>> + * revision of 1.
>> + */
>> +if (new_features & NOTMUCH_FEATURE_LAST_MOD)
>> +_notmuch_message_upgrade_last_mod (message);
>> [..]
>> +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
>> + * must call _notmuch_message_sync. */
>> +void
>> +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
>> +{
>> +/* _notmuch_message_sync will update the last modification
>> + * revision; we just have to ask it to. */
>> +message->modified = TRUE;
>> +}
>> +
>
> The comment in the first part says that message without LAST_MOD get a
> revision of 1, but as far as I can tell, _notmuch_message_sync will
> actually put the next revision number on the message. If this is what's
> happening, either the comment or the behavior should be changed,
> depending on what's intended.

I think the behaviour is OK, but you're correct the comment is
wrong. I'll see if Austin has any comment on that. I guess it would be
harmless to initialize upgraded messages to some low revision number,
but I don't see the benefit.

[PATCH 2/6] lib: Add per-message last modification tracking

[Daniel Schoepe]

Hi,

On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
> + /* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
> +  * track modification revisions.  Give all messages a
> +  * revision of 1.
> +  */
> + if (new_features & NOTMUCH_FEATURE_LAST_MOD)
> + _notmuch_message_upgrade_last_mod (message);
> [..]
> +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
> + * must call _notmuch_message_sync. */
> +void
> +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
> +{
> +/* _notmuch_message_sync will update the last modification
> + * revision; we just have to ask it to. */
> +message->modified = TRUE;
> +}
> +

The comment in the first part says that message without LAST_MOD get a
revision of 1, but as far as I can tell, _notmuch_message_sync will
actually put the next revision number on the message. If this is what's
happening, either the comment or the behavior should be changed,
depending on what's intended.

Best regards,
Daniel

Re: [PATCH 2/6] lib: Add per-message last modification tracking

[David Bremner]

Daniel Schoepe dan...@schoepe.org writes:


 On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
 +/* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
 + * track modification revisions.  Give all messages a
 + * revision of 1.
 + */
 +if (new_features  NOTMUCH_FEATURE_LAST_MOD)
 +_notmuch_message_upgrade_last_mod (message);
 [..]
 +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
 + * must call _notmuch_message_sync. */
 +void
 +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
 +{
 +/* _notmuch_message_sync will update the last modification
 + * revision; we just have to ask it to. */
 +message-modified = TRUE;
 +}
 +

 The comment in the first part says that message without LAST_MOD get a
 revision of 1, but as far as I can tell, _notmuch_message_sync will
 actually put the next revision number on the message. If this is what's
 happening, either the comment or the behavior should be changed,
 depending on what's intended.

I think the behaviour is OK, but you're correct the comment is
wrong. I'll see if Austin has any comment on that. I guess it would be
harmless to initialize upgraded messages to some low revision number,
but I don't see the benefit.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/6] lib: Add per-message last modification tracking

[Daniel Schoepe]

Hi,

On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
 + /* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
 +  * track modification revisions.  Give all messages a
 +  * revision of 1.
 +  */
 + if (new_features  NOTMUCH_FEATURE_LAST_MOD)
 + _notmuch_message_upgrade_last_mod (message);
 [..]
 +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
 + * must call _notmuch_message_sync. */
 +void
 +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
 +{
 +/* _notmuch_message_sync will update the last modification
 + * revision; we just have to ask it to. */
 +message-modified = TRUE;
 +}
 +

The comment in the first part says that message without LAST_MOD get a
revision of 1, but as far as I can tell, _notmuch_message_sync will
actually put the next revision number on the message. If this is what's
happening, either the comment or the behavior should be changed,
depending on what's intended.

Best regards,
Daniel
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/6] lib: Add per-message last modification tracking

[David Bremner]

From: Austin Clements 

This adds a new document value that stores the revision of the last
modification to message metadata, where the revision number increases
monotonically with each database commit.

An alternative would be to store the wall-clock time of the last
modification of each message.  In principle this is simpler and has
the advantage that any process can determine the current timestamp
without support from libnotmuch.  However, even assuming a computer's
clock never goes backward and ignoring clock skew in networked
environments, this has a fatal flaw.  Xapian uses (optimistic)
snapshot isolation, which means reads can be concurrent with writes.
Given this, consider the following time line with a write and two read
transactions:

   write  |-X-A--|
   read 1   |---B---|
   read 2  |---|

The write transaction modifies message X and records the wall-clock
time of the modification at A.  The writer hangs around for a while
and later commits its change.  Read 1 is concurrent with the write, so
it doesn't see the change to X.  It does some query and records the
wall-clock time of its results at B.  Transaction read 2 later starts
after the write commits and queries for changes since wall-clock time
B (say the reads are performing an incremental backup).  Even though
read 1 could not see the change to X, read 2 is told (correctly) that
X has not changed since B, the time of the last read.  In fact, X
changed before wall-clock time A, but the change was not visible until
*after* wall-clock time B, so read 2 misses the change to X.

This is tricky to solve in full-blown snapshot isolation, but because
Xapian serializes writes, we can use a simple, monotonically
increasing database revision number.  Furthermore, maintaining this
revision number requires no more IO than a wall-clock time solution
because Xapian already maintains statistics on the upper (and lower)
bound of each value stream.
---
 lib/database-private.h | 16 +++-
 lib/database.cc| 49 +++--
 lib/message.cc | 22 ++
 lib/notmuch-private.h  | 10 +-
 4 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index 24243db..5c5a2bb 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -100,6 +100,12 @@ enum _notmuch_features {
  *
  * Introduced: version 3. */
 NOTMUCH_FEATURE_INDEXED_MIMETYPES = 1 << 5,
+
+/* If set, messages store the revision number of the last
+ * modification in NOTMUCH_VALUE_LAST_MOD.
+ *
+ * Introduced: version 3. */
+NOTMUCH_FEATURE_LAST_MOD = 1 << 6,
 };

 /* In C++, a named enum is its own type, so define bitwise operators
@@ -145,6 +151,8 @@ struct _notmuch_database {

 notmuch_database_mode_t mode;
 int atomic_nesting;
+/* TRUE if changes have been made in this atomic section */
+notmuch_bool_t atomic_dirty;
 Xapian::Database *xapian_db;

 /* Bit mask of features used by this database.  This is a
@@ -158,6 +166,11 @@ struct _notmuch_database {
  * next library call. May be NULL */
 char *status_string;

+/* Highest committed revision number.  Modifications are recorded
+ * under a higher revision number, which can be generated with
+ * notmuch_database_new_revision. */
+unsigned long revision;
+
 Xapian::QueryParser *query_parser;
 Xapian::TermGenerator *term_gen;
 Xapian::ValueRangeProcessor *value_range_processor;
@@ -179,7 +192,8 @@ struct _notmuch_database {
  * will have it). */
 #define NOTMUCH_FEATURES_CURRENT \
 (NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_DIRECTORY_DOCS | \
- NOTMUCH_FEATURE_BOOL_FOLDER | NOTMUCH_FEATURE_GHOSTS)
+ NOTMUCH_FEATURE_BOOL_FOLDER | NOTMUCH_FEATURE_GHOSTS | \
+ NOTMUCH_FEATURE_LAST_MOD)

 /* Return the list of terms from the given iterator matching a prefix.
  * The prefix will be stripped from the strings in the returned list.
diff --git a/lib/database.cc b/lib/database.cc
index 78a24f7..a68a487 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -101,6 +101,9 @@ typedef struct {
  *
  * SUBJECT:The value of the "Subject" header
  *
+ * LAST_MOD:   The revision number as of the last tag or
+ * filename change.
+ *
  * In addition, terms from the content of the message are added with
  * "from", "to", "attachment", and "subject" prefixes for use by the
  * user in searching. Similarly, terms from the path of the mail
@@ -310,6 +313,8 @@ static const struct {
  * them. */
 { NOTMUCH_FEATURE_INDEXED_MIMETYPES,
   "indexed MIME types", "w"},
+{ NOTMUCH_FEATURE_LAST_MOD,
+  "modification tracking", "w"},
 };

 const char *
@@ -729,6 +734,23 @@ _notmuch_database_ensure_writable (notmuch_database_t 
*notmuch)
 return NOTMUCH_STATUS_SUCCESS;
 }

+/* Allocate a revision number for the next