[PATCH 0/2] searchidx: fix ghost vivification

[PATCH 0/2] searchidx: fix ghost vivification

[Eric Wong]

These fix a message threading bug due to out-of-order message
delivery, leading messages not showing up properly in thread
views.  Unfortunately this currently requires an index update
which cannot be done in place (yet).

Eric Wong (2):
      searchidx: simplify ghost creation
      searchidx: merge old thread id from ghosts

 lib/PublicInbox/Search.pm    |  3 ++-
 lib/PublicInbox/SearchIdx.pm | 23 ++++++-----------------
 2 files changed, 8 insertions(+), 18 deletions(-)

[PATCH 1/2] searchidx: simplify ghost creation

[Eric Wong]

Remove some worthless parameters and redundant no-ops
to make the next (important) patch easier-to-review.
---
 lib/PublicInbox/SearchIdx.pm | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index a259d86..3134687 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -150,11 +150,11 @@ sub add_message {
 
 		if ($was_ghost) {
 			$doc_id = $smsg->doc_id;
-			$self->link_message($smsg, 0);
+			$self->link_message($smsg);
 			$doc->set_data($smsg->to_doc_data);
 			$db->replace_document($doc_id, $doc);
 		} else {
-			$self->link_message($smsg, 0);
+			$self->link_message($smsg);
 			$doc->set_data($smsg->to_doc_data);
 			$doc_id = $db->add_document($doc);
 		}
@@ -211,16 +211,6 @@ sub next_thread_id {
 }
 
 sub link_message {
-	my ($self, $smsg, $is_ghost) = @_;
-
-	if ($is_ghost) {
-		$smsg->ensure_metadata;
-	} else {
-		$self->link_message_to_parents($smsg);
-	}
-}
-
-sub link_message_to_parents {
 	my ($self, $smsg) = @_;
 	my $doc = $smsg->{doc};
 	my $mid = $smsg->mid;
@@ -414,17 +404,15 @@ sub _resolve_mid_to_tid {
 }
 
 sub create_ghost {
-	my ($self, $mid, $tid) = @_;
-
-	$tid = $self->next_thread_id unless defined $tid;
+	my ($self, $mid) = @_;
 
+	my $tid = $self->next_thread_id;
 	my $doc = Search::Xapian::Document->new;
 	$doc->add_term(xpfx('mid') . $mid);
 	$doc->add_term(xpfx('thread') . $tid);
 	$doc->add_term(xpfx('type') . 'ghost');
 
 	my $smsg = PublicInbox::SearchMsg->wrap($doc, $mid);
-	$self->link_message($smsg, 1);
 	$self->{xdb}->add_document($doc);
 
 	$smsg;

[PATCH 2/2] searchidx: merge old thread id from ghosts

[Eric Wong]

We failed to discard old thread IDs when vivifying ghosts
due to out-of-order message arrival.  This rectifies the
failure and will trigger a re-index.
---
 lib/PublicInbox/Search.pm    | 3 ++-
 lib/PublicInbox/SearchIdx.pm | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index 8c0bab1..bf50365 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -36,7 +36,8 @@ use constant {
 	# 8 - remove redundant/unneeded document data
 	# 9 - disable Message-ID compression (SHA-1)
 	# 10 - optimize doc for NNTP overviews
-	SCHEMA_VERSION => 10,
+	# 11 - merge threads when vivifying ghosts
+	SCHEMA_VERSION => 11,
 
 	# n.b. FLAG_PURE_NOT is expensive not suitable for a public website
 	# as it could become a denial-of-service vector
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 3134687..58eccc1 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -150,7 +150,7 @@ sub add_message {
 
 		if ($was_ghost) {
 			$doc_id = $smsg->doc_id;
-			$self->link_message($smsg);
+			$self->link_message($smsg, $smsg->thread_id);
 			$doc->set_data($smsg->to_doc_data);
 			$db->replace_document($doc_id, $doc);
 		} else {
@@ -211,7 +211,7 @@ sub next_thread_id {
 }
 
 sub link_message {
-	my ($self, $smsg) = @_;
+	my ($self, $smsg, $old_tid) = @_;
 	my $doc = $smsg->{doc};
 	my $mid = $smsg->mid;
 	my $mime = $smsg->mime;
@@ -247,6 +247,7 @@ sub link_message {
 		# but we can never trust clients to do the right thing
 		my $ref = shift @refs;
 		$tid = $self->_resolve_mid_to_tid($ref);
+		$self->merge_threads($tid, $old_tid) if defined $old_tid;
 
 		# the rest of the refs should point to this tid:
 		foreach $ref (@refs) {

Re: [PATCH 2/2] searchidx: merge old thread id from ghosts

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> due to out-of-order message arrival.  This rectifies the
> failure and will trigger a re-index.

Btw, this reindexing is very time and space-consuming
so there needs to be a way to reindex in-place as long
as the Xapian document data is not broken...

> +++ b/lib/PublicInbox/Search.pm
> @@ -36,7 +36,8 @@ use constant {
>  	# 8 - remove redundant/unneeded document data
>  	# 9 - disable Message-ID compression (SHA-1)
>  	# 10 - optimize doc for NNTP overviews
> -	SCHEMA_VERSION => 10,
> +	# 11 - merge threads when vivifying ghosts
> +	SCHEMA_VERSION => 11,