unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [WIP] extindex: support --dedupe[=MSGID]
@ 2021-07-22  8:50 Eric Wong
  2021-07-24  6:34 ` Eric Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Wong @ 2021-07-22  8:50 UTC (permalink / raw)
  To: meta

This patch appears to do what its supposed to.  But it also
appears to be finding duplicates that were previously missed.
That's a good thing, but I wish I understood what I seem to have
fixed :x

I'm not sure why the previous ExtSearchIdx.pm (blob 357312b8)
was causing messages to be missed, even, and why this patch
seems to fix it...  And it's not infinite looping, either.

Anyways, before this patch, "-extindex --dedupe" was taking ~5
min to no-op every message (after the initial full --dedupe run
which took over a day to run).  Now that it appears to be doing
many more dedupes, so it could take 40-ish hours to finish the
dedupe run on the machine running <https://yhbt.net/lore/all/>.

Hopefully this will result in "BUG?" messages from View.pm going
away after it finishes.

-------8<------
Subject: [PATCH] extindex: support --dedupe[=MSGID]

Sometimes I just want to dedupe a single Message-ID to test
something, and this lets me do it.
---
 lib/PublicInbox/ExtSearchIdx.pm | 24 +++++++++++++++++-------
 script/public-inbox-extindex    |  4 ++--
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 357312b8..442ded46 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -887,23 +887,31 @@ sub dd_smsg { # git->cat_async callback
 	}
 }
 
-sub eidx_dedupe ($$) {
-	my ($self, $sync) = @_;
+sub eidx_dedupe ($$$) {
+	my ($self, $sync, $msgids) = @_;
 	$sync->{dedupe_cull} = 0;
 	my $candidates = 0;
 	my $nr_mid = 0;
 	return unless eidxq_lock_acquire($self);
-	my $iter;
+	my ($iter, $cur_mid);
 	my $min_id = 0;
 	local $sync->{-regen_fmt} = "dedupe %u/".$self->{oidx}->max."\n";
 
 	# note: we could write this query more intelligently,
 	# but that causes lock contention with read-only processes
 dedupe_restart:
-	$iter = $self->{oidx}->dbh->prepare(<<EOS);
+	$cur_mid = shift @$msgids;
+	if ($cur_mid eq '') { # all Message-IDs
+		$iter = $self->{oidx}->dbh->prepare(<<EOS);
 SELECT mid,id FROM msgid WHERE id > ? ORDER BY id ASC
 EOS
-	$iter->execute($min_id);
+		$iter->execute($min_id);
+	} else {
+		$iter = $self->{oidx}->dbh->prepare(<<EOS);
+SELECT mid,id FROM msgid WHERE mid = ? AND id > ? ORDER BY id ASC
+EOS
+		$iter->execute($cur_mid, $min_id);
+	}
 	while (my ($mid, $id) = $iter->fetchrow_array) {
 		last if $sync->{quit};
 		$self->{current_info} = "dedupe $mid";
@@ -937,6 +945,8 @@ EOS
 			goto dedupe_restart;
 		}
 	}
+	goto dedupe_restart if scalar(@$msgids);
+
 	my $n = delete $sync->{dedupe_cull};
 	if (my $pr = $sync->{-opt}->{-progress}) {
 		$pr->("culled $n/$candidates candidates ($nr_mid msgids)\n");
@@ -974,9 +984,9 @@ sub eidx_sync { # main entry point
 	for my $ibx (@{ibx_sorted($self)}) {
 		$ibx->{-ibx_id} //= $self->{oidx}->ibx_id($ibx->eidx_key);
 	}
-	if (delete($opt->{dedupe})) {
+	if (my $msgids = delete($opt->{dedupe})) {
 		local $sync->{checkpoint_unlocks} = 1;
-		eidx_dedupe($self, $sync);
+		eidx_dedupe($self, $sync, $msgids);
 	}
 	if (delete($opt->{reindex})) {
 		local $sync->{checkpoint_unlocks} = 1;
diff --git a/script/public-inbox-extindex b/script/public-inbox-extindex
index dcb12e5a..addd5ac6 100755
--- a/script/public-inbox-extindex
+++ b/script/public-inbox-extindex
@@ -17,7 +17,7 @@ usage: public-inbox-extindex [options] [EXTINDEX_DIR] [INBOX_DIR...]
   --batch-size=BYTES  flush changes to OS after a given number of bytes
   --max-size=BYTES    do not index messages larger than the given size
   --gc                perform garbage collection instead of indexing
-  --dedupe            fix prior deduplication errors
+  --dedupe[=MSGID]    fix prior deduplication errors (may be repeated)
   --verbose | -v      increase verbosity (may be repeated)
   --dry-run | -n      dry-run on --dedupe
 
@@ -29,7 +29,7 @@ GetOptions($opt, qw(verbose|v+ reindex rethread compact|c+ jobs|j=i
 		fsync|sync!
 		indexlevel|index-level|L=s max_size|max-size=s
 		batch_size|batch-size=s
-		dedupe gc commit-interval=i watch scan! dry-run|n
+		dedupe:s@ gc commit-interval=i watch scan! dry-run|n
 		all help|h))
 	or die $help;
 if ($opt->{help}) { print $help; exit 0 };

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [WIP] extindex: support --dedupe[=MSGID]
  2021-07-22  8:50 [WIP] extindex: support --dedupe[=MSGID] Eric Wong
@ 2021-07-24  6:34 ` Eric Wong
  2021-07-25  0:11   ` [PATCH 0/3] extindex dedupe improvements Eric Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Wong @ 2021-07-24  6:34 UTC (permalink / raw)
  To: meta

Eric Wong <e@80x24.org> wrote:
> Anyways, before this patch, "-extindex --dedupe" was taking ~5
> min to no-op every message (after the initial full --dedupe run
> which took over a day to run).  Now that it appears to be doing
> many more dedupes, so it could take 40-ish hours to finish the
> dedupe run on the machine running <https://yhbt.net/lore/all/>.

Nearly 44 hours (on a busy system), but that was only because of
a bug which caused half the work to get skipped (fix below).

Xapian glass (1.4.x) really falls down with large shards and
making -xcpdb work with extindex to reshard into more shards
will be required to maintain performance as an extindex grows...

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 442ded46..2311161e 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -895,12 +895,13 @@ sub eidx_dedupe ($$$) {
 	return unless eidxq_lock_acquire($self);
 	my ($iter, $cur_mid);
 	my $min_id = 0;
+	my $idx = 0;
 	local $sync->{-regen_fmt} = "dedupe %u/".$self->{oidx}->max."\n";
 
 	# note: we could write this query more intelligently,
 	# but that causes lock contention with read-only processes
 dedupe_restart:
-	$cur_mid = shift @$msgids;
+	$cur_mid = $msgids->[$idx];
 	if ($cur_mid eq '') { # all Message-IDs
 		$iter = $self->{oidx}->dbh->prepare(<<EOS);
 SELECT mid,id FROM msgid WHERE id > ? ORDER BY id ASC
@@ -945,7 +946,7 @@ EOS
 			goto dedupe_restart;
 		}
 	}
-	goto dedupe_restart if scalar(@$msgids);
+	goto dedupe_restart if defined($msgids->[++$idx]);
 
 	my $n = delete $sync->{dedupe_cull};
 	if (my $pr = $sync->{-opt}->{-progress}) {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 0/3] extindex dedupe improvements
  2021-07-24  6:34 ` Eric Wong
@ 2021-07-25  0:11   ` Eric Wong
  2021-07-25  0:11     ` [PATCH 1/3] extindex: support --dedupe[=MSGID] Eric Wong
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Eric Wong @ 2021-07-25  0:11 UTC (permalink / raw)
  To: meta

It's still slow as hell due to I/O latency from Xapian(glass)
on RAM-starved systems; but at least dedupe seems correct, now.

Eric Wong (3):
  extindex: support --dedupe[=MSGID]
  extindex: improve comment around git->async_wait_all
  extsearchidx: use more appropriate max for dedupe

 lib/PublicInbox/ExtSearchIdx.pm | 34 ++++++++++++++++++++++++---------
 script/public-inbox-extindex    |  4 ++--
 2 files changed, 27 insertions(+), 11 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] extindex: support --dedupe[=MSGID]
  2021-07-25  0:11   ` [PATCH 0/3] extindex dedupe improvements Eric Wong
@ 2021-07-25  0:11     ` Eric Wong
  2021-07-25  0:11     ` [PATCH 2/3] extindex: improve comment around git->async_wait_all Eric Wong
  2021-07-25  0:11     ` [PATCH 3/3] extsearchidx: use more appropriate max for dedupe Eric Wong
  2 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-07-25  0:11 UTC (permalink / raw)
  To: meta

Sometimes I just want to dedupe a single Message-ID to test
something, and this lets me do it.

This patch appears to do what its supposed to.  But it also
appears to be finding duplicates that were previously missed.
That's a good thing, but I wish I understood what seems to be
fixed :x

I'm not sure why the previous ExtSearchIdx.pm (blob 357312b8)
was causing messages to be missed, even, and why this patch
seems to fix it...  And it's not infinite looping, either.

Anyways, before this patch, "-extindex --dedupe" was taking ~5
min to no-op every message (after the initial full --dedupe run
which took over a day to run).  No-op --dedupes now take just
under 2 hours to scan every single cross-posted message for a
no-op dedupe.  The initial dedupe took nearly 44 hours on my
system for <https://yhbt.net/lore/all/> due to SATA-2 TLC SSD
latency on 3 gigantic Xapian shards.

Running --dedupe with this change seems to prevent
/BUG\?.*?not deduplicated properly/ stderr messages from being
triggered by View.pm.  Current versions of -extindex do not
seem susceptible to introducing duplicates.
---
 lib/PublicInbox/ExtSearchIdx.pm | 25 ++++++++++++++++++-------
 script/public-inbox-extindex    |  4 ++--
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 357312b8..2311161e 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -887,23 +887,32 @@ sub dd_smsg { # git->cat_async callback
 	}
 }
 
-sub eidx_dedupe ($$) {
-	my ($self, $sync) = @_;
+sub eidx_dedupe ($$$) {
+	my ($self, $sync, $msgids) = @_;
 	$sync->{dedupe_cull} = 0;
 	my $candidates = 0;
 	my $nr_mid = 0;
 	return unless eidxq_lock_acquire($self);
-	my $iter;
+	my ($iter, $cur_mid);
 	my $min_id = 0;
+	my $idx = 0;
 	local $sync->{-regen_fmt} = "dedupe %u/".$self->{oidx}->max."\n";
 
 	# note: we could write this query more intelligently,
 	# but that causes lock contention with read-only processes
 dedupe_restart:
-	$iter = $self->{oidx}->dbh->prepare(<<EOS);
+	$cur_mid = $msgids->[$idx];
+	if ($cur_mid eq '') { # all Message-IDs
+		$iter = $self->{oidx}->dbh->prepare(<<EOS);
 SELECT mid,id FROM msgid WHERE id > ? ORDER BY id ASC
 EOS
-	$iter->execute($min_id);
+		$iter->execute($min_id);
+	} else {
+		$iter = $self->{oidx}->dbh->prepare(<<EOS);
+SELECT mid,id FROM msgid WHERE mid = ? AND id > ? ORDER BY id ASC
+EOS
+		$iter->execute($cur_mid, $min_id);
+	}
 	while (my ($mid, $id) = $iter->fetchrow_array) {
 		last if $sync->{quit};
 		$self->{current_info} = "dedupe $mid";
@@ -937,6 +946,8 @@ EOS
 			goto dedupe_restart;
 		}
 	}
+	goto dedupe_restart if defined($msgids->[++$idx]);
+
 	my $n = delete $sync->{dedupe_cull};
 	if (my $pr = $sync->{-opt}->{-progress}) {
 		$pr->("culled $n/$candidates candidates ($nr_mid msgids)\n");
@@ -974,9 +985,9 @@ sub eidx_sync { # main entry point
 	for my $ibx (@{ibx_sorted($self)}) {
 		$ibx->{-ibx_id} //= $self->{oidx}->ibx_id($ibx->eidx_key);
 	}
-	if (delete($opt->{dedupe})) {
+	if (my $msgids = delete($opt->{dedupe})) {
 		local $sync->{checkpoint_unlocks} = 1;
-		eidx_dedupe($self, $sync);
+		eidx_dedupe($self, $sync, $msgids);
 	}
 	if (delete($opt->{reindex})) {
 		local $sync->{checkpoint_unlocks} = 1;
diff --git a/script/public-inbox-extindex b/script/public-inbox-extindex
index dcb12e5a..addd5ac6 100755
--- a/script/public-inbox-extindex
+++ b/script/public-inbox-extindex
@@ -17,7 +17,7 @@ usage: public-inbox-extindex [options] [EXTINDEX_DIR] [INBOX_DIR...]
   --batch-size=BYTES  flush changes to OS after a given number of bytes
   --max-size=BYTES    do not index messages larger than the given size
   --gc                perform garbage collection instead of indexing
-  --dedupe            fix prior deduplication errors
+  --dedupe[=MSGID]    fix prior deduplication errors (may be repeated)
   --verbose | -v      increase verbosity (may be repeated)
   --dry-run | -n      dry-run on --dedupe
 
@@ -29,7 +29,7 @@ GetOptions($opt, qw(verbose|v+ reindex rethread compact|c+ jobs|j=i
 		fsync|sync!
 		indexlevel|index-level|L=s max_size|max-size=s
 		batch_size|batch-size=s
-		dedupe gc commit-interval=i watch scan! dry-run|n
+		dedupe:s@ gc commit-interval=i watch scan! dry-run|n
 		all help|h))
 	or die $help;
 if ($opt->{help}) { print $help; exit 0 };

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] extindex: improve comment around git->async_wait_all
  2021-07-25  0:11   ` [PATCH 0/3] extindex dedupe improvements Eric Wong
  2021-07-25  0:11     ` [PATCH 1/3] extindex: support --dedupe[=MSGID] Eric Wong
@ 2021-07-25  0:11     ` Eric Wong
  2021-07-25  0:11     ` [PATCH 3/3] extsearchidx: use more appropriate max for dedupe Eric Wong
  2 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-07-25  0:11 UTC (permalink / raw)
  To: meta

I found myself tempted to remove this, but it appears impossible
due to odd messages which have multiple Message-IDs.
---
 lib/PublicInbox/ExtSearchIdx.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 2311161e..1c2a9758 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -937,7 +937,9 @@ EOS
 			};
 			$self->git->cat_async($smsg->{blob}, \&dd_smsg, $dd);
 		}
-		# need to wait on every single one
+		# need to wait on every single one @smsg contents can get
+		# invalidated inside dd_smsg for messages with multiple
+		# Message-IDs.
 		$self->git->async_wait_all;
 
 		if (checkpoint_due($sync)) {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] extsearchidx: use more appropriate max for dedupe
  2021-07-25  0:11   ` [PATCH 0/3] extindex dedupe improvements Eric Wong
  2021-07-25  0:11     ` [PATCH 1/3] extindex: support --dedupe[=MSGID] Eric Wong
  2021-07-25  0:11     ` [PATCH 2/3] extindex: improve comment around git->async_wait_all Eric Wong
@ 2021-07-25  0:11     ` Eric Wong
  2 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-07-25  0:11 UTC (permalink / raw)
  To: meta

The over.msgid table may contain ghost Message-IDs and also
Message-IDs of deleted spam messages, so over->max isn't a
good aproproximation of dedupe progress.
---
 lib/PublicInbox/ExtSearchIdx.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 1c2a9758..51dbf54f 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -896,7 +896,10 @@ sub eidx_dedupe ($$$) {
 	my ($iter, $cur_mid);
 	my $min_id = 0;
 	my $idx = 0;
-	local $sync->{-regen_fmt} = "dedupe %u/".$self->{oidx}->max."\n";
+	my ($max_id) = $self->{oidx}->dbh->selectrow_array(<<EOS);
+SELECT MAX(id) FROM msgid
+EOS
+	local $sync->{-regen_fmt} = "dedupe %u/$max_id\n";
 
 	# note: we could write this query more intelligently,
 	# but that causes lock contention with read-only processes

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-25  0:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22  8:50 [WIP] extindex: support --dedupe[=MSGID] Eric Wong
2021-07-24  6:34 ` Eric Wong
2021-07-25  0:11   ` [PATCH 0/3] extindex dedupe improvements Eric Wong
2021-07-25  0:11     ` [PATCH 1/3] extindex: support --dedupe[=MSGID] Eric Wong
2021-07-25  0:11     ` [PATCH 2/3] extindex: improve comment around git->async_wait_all Eric Wong
2021-07-25  0:11     ` [PATCH 3/3] extsearchidx: use more appropriate max for dedupe Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).