search: simplify unindexing

Since over.sqlite3 seems here to stay, we no longer need to do
Message-ID lookups against Xapian and can simply rely on the
docid <=> NNTP article number equivalancy SCHEMA_VERSION=15
gave us.

This rids us of the closure-using batch_do sub in the v1
code path and vastly simplifies both v1 and v2 unindexing.

diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 29c6e0b933aae2e757f6307ec3758c53fc1dec17..5601e602c0181aebfd89d01d541d51fa2ac05b64 100644 (file)
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -326,22 +326,23 @@ INSERT INTO id2num (id, num) VALUES (?,?)
 }
 
 sub _remove_oid {
-       my ($self, $smsg, $oid, $nr) = @_;
+       my ($self, $smsg, $oid, $removed) = @_;
        if (!defined($oid) || $smsg->{blob} eq $oid) {
                delete_by_num($self, $smsg->{num});
-               $$nr++;
+               push @$removed, $smsg->{num};
        }
        1;
 }
 
-# returns number of removed messages
+# returns number of removed messages in scalar context,
+# array of removed article numbers in array context.
 # $oid may be undef to match only on $mid
 sub remove_oid {
        my ($self, $oid, $mid) = @_;
-       my $nr = 0;
+       my $removed = [];
        begin_lazy($self);
-       each_by_mid($self, $mid, ['ddd'], \&_remove_oid, $oid, \$nr);
-       $nr;
+       each_by_mid($self, $mid, ['ddd'], \&_remove_oid, $oid, $removed);
+       @$removed;
 }
 
 sub _num_mid0_for_oid {

diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 9550847b784c42d96da03e44688c377d20c1be71..8316250906994e6649d1a96cf80edb0197e18aa8 100644 (file)
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -12,7 +12,7 @@ use v5.10.1;
 use parent qw(PublicInbox::Search PublicInbox::Lock);
 use PublicInbox::Eml;
 use PublicInbox::InboxWritable;
-use PublicInbox::MID qw/mid_clean mid_mime mids_for_index/;
+use PublicInbox::MID qw(mid_mime mids_for_index mids);
 use PublicInbox::MsgIter;
 use Carp qw(croak);
 use POSIX qw(strftime);
@@ -413,88 +413,32 @@ sub add_message {
        $smsg->{num};
 }
 
-# returns begin and end PostingIterator
-sub find_doc_ids {
-       my ($self, $termval) = @_;
-       my $db = $self->{xdb};
-
-       ($db->postlist_begin($termval), $db->postlist_end($termval));
-}
-
-# v1 only
-sub batch_do {
-       my ($self, $termval, $cb) = @_;
-       my $batch_size = 1000; # don't let @ids grow too large to avoid OOM
-       while (1) {
-               my ($head, $tail) = $self->find_doc_ids($termval);
-               return if $head == $tail;
-               my @ids;
-               for (; $head != $tail && @ids < $batch_size; $head++) {
-                       push @ids, $head->get_docid;
+sub xdb_remove {
+       my ($self, $oid, @removed) = @_;
+       my $xdb = $self->{xdb} or return;
+       for my $num (@removed) {
+               my $doc = eval { $xdb->get_document($num) };
+               unless ($doc) {
+                       warn "E: $@\n" if $@;
+                       warn "E: #$num $oid missing in Xapian\n";
+                       next;
                }
-               $cb->(\@ids);
-       }
-}
-
-# v1 only, where $mid is unique
-sub remove_message {
-       my ($self, $mid) = @_;
-       $mid = mid_clean($mid);
-
-       if (my $over = $self->{over}) {
-               my $nr = eval { $over->remove_oid(undef, $mid) };
-               if ($@) {
-                       warn "failed to remove <$mid> from overview: $@\n";
-               } elsif ($nr == 0) {
-                       warn "<$mid> missing for removal from overview\n";
+               my $smsg = bless {}, 'PublicInbox::Smsg';
+               $smsg->load_expand($doc);
+               my $blob = $smsg->{blob} // '(unset)';
+               if ($blob eq $oid) {
+                       $xdb->delete_document($num);
+               } else {
+                       warn "E: #$num $oid != $blob in Xapian\n";
                }
        }
-       return unless need_xapian($self);
-       my $db = $self->{xdb};
-       my $nr = 0;
-       eval {
-               batch_do($self, 'Q' . $mid, sub {
-                       my ($ids) = @_;
-                       $db->delete_document($_) for @$ids;
-                       $nr += scalar @$ids;
-               });
-       };
-       if ($@) {
-               warn "failed to remove <$mid> from Xapian: $@\n";
-       } elsif ($nr == 0) {
-               warn "<$mid> missing for removal from Xapian\n";
-       }
 }
 
-# MID is a hint in V2
 sub remove_by_oid {
-       my ($self, $oid, $mid) = @_;
-
-       $self->{over}->remove_oid($oid, $mid) if $self->{over};
-
-       return unless need_xapian($self);
-       my $db = $self->{xdb};
-
-       # XXX careful, we cannot use batch_do here since we conditionally
-       # delete documents based on other factors, so we cannot call
-       # find_doc_ids twice.
-       my ($head, $tail) = $self->find_doc_ids('Q' . $mid);
-       return if $head == $tail;
-
-       # there is only ONE element in @delete unless we
-       # have bugs in our v2writable deduplication check
-       my @delete;
-       for (; $head != $tail; $head++) {
-               my $docid = $head->get_docid;
-               my $doc = $db->get_document($docid);
-               my $smsg = bless { mid => $mid }, 'PublicInbox::Smsg';
-               $smsg->load_expand($doc);
-               if ($smsg->{blob} eq $oid) {
-                       push(@delete, $docid);
-               }
-       }
-       $db->delete_document($_) foreach @delete;
-       scalar(@delete);
+       my ($self, $oid, $num) = @_;
+       die "BUG: remove_by_oid is v2-only\n" if $self->{over};
+       $self->begin_txn_lazy;
+       xdb_remove($self, $oid, $num) if need_xapian($self);
 }
 
 sub index_git_blob_id {
@@ -507,10 +451,29 @@ sub index_git_blob_id {
        }
 }
 
-sub unindex_blob {
-       my ($self, $mime) = @_;
-       my $mid = eval { mid_mime($mime) };
-       $self->remove_message($mid) if defined $mid;
+# v1 only
+sub unindex_eml {
+       my ($self, $oid, $eml) = @_;
+       my $mids = mids($eml);
+       my $nr = 0;
+       my %tmp;
+       for my $mid (@$mids) {
+               my @removed = eval { $self->{over}->remove_oid($oid, $mid) };
+               if ($@) {
+                       warn "E: failed to remove <$mid> from overview: $@\n";
+               } else {
+                       $nr += scalar @removed;
+                       $tmp{$_}++ for @removed;
+               }
+       }
+       if (!$nr) {
+               $mids = join('> <', @$mids);
+               warn "W: <$mids> missing for removal from overview\n";
+       }
+       while (my ($num, $nr) = each %tmp) {
+               warn "BUG: $num appears >1 times ($nr) for $oid\n" if $nr != 1;
+       }
+       xdb_remove($self, $oid, keys %tmp) if need_xapian($self);
 }
 
 sub index_mm {
@@ -577,7 +540,7 @@ sub index_both { # git->cat_async callback
 sub unindex_both { # git->cat_async callback
        my ($bref, $oid, $type, $size, $self) = @_;
        my $eml = PublicInbox::Eml->new($bref);
-       unindex_blob($self, $eml);
+       unindex_eml($self, $oid, $eml);
        unindex_mm($self, $eml);
 }
 
@@ -844,13 +807,12 @@ sub remote_close {
 }
 
 sub remote_remove {
-       my ($self, $oid, $mid) = @_;
+       my ($self, $oid, $num) = @_;
        if (my $w = $self->{w}) {
                # triggers remove_by_oid in a shard
-               print $w "D $oid $mid\n" or die "failed to write remove $!";
+               print $w "D $oid $num\n" or die "failed to write remove $!";
        } else {
-               $self->begin_txn_lazy;
-               $self->remove_by_oid($oid, $mid);
+               $self->remove_by_oid($oid, $num);
        }
 }
 

diff --git a/lib/PublicInbox/SearchIdxShard.pm b/lib/PublicInbox/SearchIdxShard.pm
index b51d148b161492bcb07d04f36e60a569d89b45d4..544268819070c675bfa9d3e2a3edc92c11ffb2a2 100644 (file)
--- a/lib/PublicInbox/SearchIdxShard.pm
+++ b/lib/PublicInbox/SearchIdxShard.pm
@@ -62,10 +62,8 @@ sub shard_worker_loop ($$$$$) {
                        # no need to lock < 512 bytes is atomic under POSIX
                        print $bnote "barrier $shard\n" or
                                        die "write failed for barrier $!\n";
-               } elsif ($line =~ /\AD ([a-f0-9]{40,}) (.+)\n\z/s) {
-                       my ($oid, $mid) = ($1, $2);
-                       $self->begin_txn_lazy;
-                       $self->remove_by_oid($oid, $mid);
+               } elsif ($line =~ /\AD ([a-f0-9]{40,}) ([0-9]+)\n\z/s) {
+                       $self->remove_by_oid($1, $2 + 0);
                } else {
                        chomp $line;
                        # n.b. $mid may contain spaces(!)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index b51c85250a9b6be8794c30ea51879f1a2f5ffca1..dffe90d8cb05b4bffe8acd7c9f914e3cd0d8f79d 100644 (file)
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -1185,8 +1185,11 @@ sub sync_prepare ($$$) {
 
 sub unindex_oid_remote ($$$) {
        my ($self, $oid, $mid) = @_;
-       $_->remote_remove($oid, $mid) foreach @{$self->{idx_shards}};
-       $self->{over}->remove_oid($oid, $mid);
+       my @removed = $self->{over}->remove_oid($oid, $mid);
+       for my $num (@removed) {
+               my $idx = idx_shard($self, $num % $self->{shards});
+               $idx->remote_remove($oid, $num);
+       }
 }
 
 sub unindex_oid ($$$;$) {

diff --git a/t/search.t b/t/search.t
index 82caf9e41c3dc5170514db48f82ac52e9602b54a..aa6f94bf348c39c874a2b90bca4a81a459d3072a 100644 (file)
--- a/t/search.t
+++ b/t/search.t
@@ -397,7 +397,9 @@ $ibx->with_umask(sub {
 
 $ibx->with_umask(sub {
        my $amsg = eml_load 't/search-amsg.eml';
-       ok($rw->add_message($amsg), 'added attachment');
+       my $oid = ('0'x40);
+       my $smsg = bless { blob => $oid }, 'PublicInbox::Smsg';
+       ok($rw->add_message($amsg, $smsg), 'added attachment');
        $rw_commit->();
        $ro->reopen;
        my $n = $ro->query('n:attached_fart.txt');
@@ -418,7 +420,7 @@ $ibx->with_umask(sub {
                $art = $ro->{over_ro}->next_by_mid($mid, \$id, \$prev);
                ok($art, 'article exists in OVER DB');
        }
-       $rw->unindex_blob($amsg);
+       $rw->unindex_eml($oid, $amsg);
        $rw->commit_txn_lazy;
        SKIP: {
                skip('$art not defined', 1) unless defined $art;