From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 6FAFA200A5 for ; Tue, 27 Oct 2020 07:55:01 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 47/52] extsearchidx: handle edits Date: Tue, 27 Oct 2020 07:54:48 +0000 Message-Id: <20201027075453.19163-48-e@80x24.org> In-Reply-To: <20201027075453.19163-1-e@80x24.org> References: <20201027075453.19163-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: We can now handle cases where messages are edited in one inbox but not another, bifurcating the message. V2Writable::log_range handles some edge-cases which could happen in v2-only code paths, as well, but weren't usually triggered due to default git-gc knobs not pruning immediately --- lib/PublicInbox/ExtSearchIdx.pm | 66 ++++++++++++++++++++++++++------- lib/PublicInbox/OverIdx.pm | 44 +++++++++++++++++----- lib/PublicInbox/V2Writable.pm | 26 ++++++++----- t/extsearch.t | 28 +++++++++++++- 4 files changed, 132 insertions(+), 32 deletions(-) diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index 026e1377..bfe39891 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -19,7 +19,7 @@ use v5.10.1; use parent qw(PublicInbox::ExtSearch PublicInbox::Lock); use Carp qw(croak carp); use PublicInbox::Search; -use PublicInbox::SearchIdx qw(crlf_adjust prepare_stack); +use PublicInbox::SearchIdx qw(crlf_adjust prepare_stack is_ancestor); use PublicInbox::OverIdx; use PublicInbox::MID qw(mids); use PublicInbox::V2Writable; @@ -119,6 +119,7 @@ sub do_xpost ($$) { $self->{oidx}->add_xref3($docid, $xnum, $oid, $xibx->eidx_key); $idx->shard_add_eidx_info($docid, $oid, $xibx, $eml); } else { # 'd' + $self->{oidx}->remove_xref3($docid, $oid, $xibx->eidx_key); $idx->shard_remove_eidx_info($docid, $oid, $xibx, $eml); } } @@ -176,14 +177,35 @@ sub do_step ($) { # main iterator for adding messages to the index do_finalize($req); } +sub _blob_missing ($) { # called when req->{cur_smsg}->{blob} is bad + my ($req) = @_; + my $smsg = $req->{cur_smsg} or die 'BUG: {cur_smsg} missing'; + my $self = $req->{self}; + my $xref3 = $self->{oidx}->get_xref3($smsg->{num}); + my @keep = grep(!/:$smsg->{blob}\z/, @$xref3); + if (@keep) { + $keep[0] =~ /:([a-f0-9]{40,}+)\z/ or + die "BUG: xref $keep[0] has no OID"; + my $oidhex = $1; + $self->{oidx}->remove_xref3($smsg->{num}, $smsg->{blob}); + my $upd = $self->{oidx}->update_blob($smsg, $oidhex); + my $saved = $self->{oidx}->get_art($smsg->{num}); + } else { + $self->{oidx}->delete_by_num($smsg->{num}); + } +} + sub ck_existing { # git->cat_async callback my ($bref, $oid, $type, $size, $req) = @_; my $smsg = $req->{cur_smsg} or die 'BUG: {cur_smsg} missing'; - return if is_bad_blob($oid, $type, $size, $smsg->{blob}); - my $cur = PublicInbox::Eml->new($bref); - if (content_hash($cur) eq $req->{chash}) { - push @{$req->{indexed}}, $smsg; # for do_xpost - } # else { index_unseen later } + if ($type eq 'missing') { + _blob_missing($req); + } elsif (!is_bad_blob($oid, $type, $size, $smsg->{blob})) { + my $cur = PublicInbox::Eml->new($bref); + if (content_hash($cur) eq $req->{chash}) { + push @{$req->{indexed}}, $smsg; # for do_xpost + } # else { index_unseen later } + } do_step($req); } @@ -281,15 +303,33 @@ sub eidx_sync { # main entry point PublicInbox::V2Writable::done($self); } -sub update_last_commit { +sub update_last_commit { # overrides V2Writable my ($self, $sync, $unit, $latest_cmt) = @_; + return unless defined $latest_cmt; - my $ALL = $self->git; - # while (scalar(@{$ALL->{inflight_c}}) || scalar(@{$ALL->{inflight}})) { - # $ALL->check_async_wait; - # $ALL->cat_async_wait; - # } - # TODO + $self->git->async_wait_all; + my $ibx = $sync->{ibx} or die 'BUG: {ibx} missing'; + my $ekey = $ibx->eidx_key; + my $uv = $ibx->uidvalidity; + my $epoch = $unit->{epoch}; + my $meta_key; + my $v = $ibx->version; + if ($v == 2) { + die 'No {epoch} for v2 unit' unless defined $epoch; + $meta_key = "lc-v2:$ekey//$uv;$epoch"; + } elsif ($v == 1) { + die 'Unexpected {epoch} for v1 unit' if defined $epoch; + $meta_key = "lc-v1:$ekey//$uv"; + } else { + die "Unsupported inbox version: $v"; + } + my $last = $self->{oidx}->eidx_meta($meta_key); + if (defined $last && is_ancestor($unit->{git}, $last, $latest_cmt)) { + my @cmd = (qw(rev-list --count), "$last..$latest_cmt"); + chomp(my $n = $unit->{git}->qx(@cmd)); + return if $n ne '' && $n == 0; + } + $self->{oidx}->eidx_meta($meta_key, $latest_cmt); } sub idx_init { # similar to V2Writable diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm index dff2780d..173e3220 100644 --- a/lib/PublicInbox/OverIdx.pm +++ b/lib/PublicInbox/OverIdx.pm @@ -261,6 +261,13 @@ sub subject_path ($) { lc($subj); } +sub ddd_for ($) { + my ($smsg) = @_; + my $dd = $smsg->to_doc_data; + utf8::encode($dd); + compress($dd); +} + sub add_overview { my ($self, $eml, $smsg) = @_; $smsg->{lines} = $eml->body_raw =~ tr!\n!\n!; @@ -272,10 +279,7 @@ sub add_overview { $xpath = subject_path($subj); $xpath = id_compress($xpath); } - my $dd = $smsg->to_doc_data; - utf8::encode($dd); - $dd = compress($dd); - add_over($self, $smsg, $mids, $refs, $xpath, $dd); + add_over($self, $smsg, $mids, $refs, $xpath, ddd_for($smsg)); } sub _add_over { @@ -589,14 +593,36 @@ INSERT OR IGNORE INTO xref3 (docid, ibx_id, xnum, oidbin) VALUES (?, ?, ?, ?) sub remove_xref3 { my ($self, $docid, $oidhex, $eidx_key) = @_; begin_lazy($self); - my $ibx_id = id_for($self, 'inboxes', 'ibx_id', eidx_key => $eidx_key); my $oidbin = pack('H*', $oidhex); - my $sth = $self->{dbh}->prepare_cached(<<''); + my $sth; + if (defined $eidx_key) { + my $ibx_id = id_for($self, 'inboxes', 'ibx_id', + eidx_key => $eidx_key); + $sth = $self->{dbh}->prepare_cached(<<''); DELETE FROM xref3 WHERE docid = ? AND ibx_id = ? AND oidbin = ? - $sth->bind_param(1, $docid); - $sth->bind_param(2, $ibx_id); - $sth->bind_param(3, $oidbin, SQL_BLOB); + $sth->bind_param(1, $docid); + $sth->bind_param(2, $ibx_id); + $sth->bind_param(3, $oidbin, SQL_BLOB); + } else { + $sth = $self->{dbh}->prepare_cached(<<''); +DELETE FROM xref3 WHERE docid = ? AND oidbin = ? + + $sth->bind_param(1, $docid); + $sth->bind_param(2, $oidbin, SQL_BLOB); + } + $sth->execute; +} + +# for when an xref3 goes missing, this does NOT update {ts} +sub update_blob { + my ($self, $smsg, $oidhex) = @_; + my $sth = $self->{dbh}->prepare(<<''); +UPDATE over SET ddd = ? WHERE num = ? + + $smsg->{blob} = $oidhex; + $sth->bind_param(1, ddd_for($smsg), SQL_BLOB); + $sth->bind_param(2, $smsg->{num}); $sth->execute; } diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index efda7907..0364857f 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -567,7 +567,7 @@ sub last_epoch_commit ($$;$) { $self->{mm}->last_commit_xap($v, $i, $cmt); } -sub set_last_commits ($) { +sub set_last_commits ($) { # this is NOT for ExtSearchIdx my ($self) = @_; defined(my $epoch_max = $self->{epoch_max}) or return; my $last_commit = $self->{last_commit}; @@ -992,9 +992,10 @@ sub log_range ($$$) { my $range = "$cur..$tip"; $pr->("$i.git checking contiguity... ") if $pr; - if (is_ancestor($unit->{git}, $cur, $tip)) { # common case + my $git = $unit->{git}; + if (is_ancestor($git, $cur, $tip)) { # common case $pr->("OK\n") if $pr; - my $n = $unit->{git}->qx(qw(rev-list --count), $range); + my $n = $git->qx(qw(rev-list --count), $range); chomp($n); if ($n == 0) { $sync->{ranges}->[$i] = undef; @@ -1006,9 +1007,9 @@ sub log_range ($$$) { $pr->("FAIL\n") if $pr; warn <<""; discontiguous range: $range -Rewritten history? (in $unit->{git}->{git_dir}) +Rewritten history? (in $git->{git_dir}) - chomp(my $base = $unit->{git}->qx('merge-base', $tip, $cur)); + chomp(my $base = $git->qx('merge-base', $tip, $cur)); if ($base) { $range = "$base..$tip"; warn "found merge-base: $base\n" @@ -1017,10 +1018,17 @@ Rewritten history? (in $unit->{git}->{git_dir}) warn "discarding history at $cur\n"; } warn <<""; -reindexing $unit->{git}->{git_dir} starting at -$range - - $unit->{unindex_range} = "$base..$cur"; +reindexing $git->{git_dir} +starting at $range + + # $cur^0 may no longer exist if pruned by git + if ($git->qx(qw(rev-parse -q --verify), "$cur^0")) { + $unit->{unindex_range} = "$base..$cur"; + } elsif ($base && $git->qx(qw(rev-parse -q --verify), $base)) { + $unit->{unindex_range} = "$base.."; + } else { + warn "W: unable to unindex before $range\n"; + } } $range; } diff --git a/t/extsearch.t b/t/extsearch.t index 108ffaeb..8d2c1507 100644 --- a/t/extsearch.t +++ b/t/extsearch.t @@ -35,8 +35,8 @@ run_script(['-index', "$home/v1test"]) or BAIL_OUT "index $?"; ok(run_script([qw(-eindex --all), "$home/eindex"]), 'eindex init'); +my $es = PublicInbox::ExtSearch->new("$home/eindex"); { - my $es = PublicInbox::ExtSearch->new("$home/eindex"); my $smsg = $es->over->get_art(1); ok($smsg, 'got first article'); is($es->over->get_art(2), undef, 'only one added'); @@ -46,4 +46,30 @@ ok(run_script([qw(-eindex --all), "$home/eindex"]), 'eindex init'); is(scalar(@$xref3), 2, 'only to entries'); } +{ + my ($in, $out, $err); + $in = $out = $err = ''; + my $opt = { 0 => \$in, 1 => \$out, 2 => \$err }; + my $env = { MAIL_EDITOR => "$^X -i -p -e 's/test message/BEST MSG/'" }; + my $cmd = [ qw(-edit -Ft/utf8.eml), "$home/v2test" ]; + ok(run_script($cmd, $env, $opt), '-edit'); + ok(run_script([qw(-eindex --all), "$home/eindex"], undef, $opt), + 'eindex again'); + like($err, qr/discontiguous range/, 'warned about discontiguous range'); + my $msg1 = $es->over->get_art(1) or BAIL_OUT 'msg1 missing'; + my $msg2 = $es->over->get_art(2) or BAIL_OUT 'msg2 missing'; + is($msg1->{mid}, $msg2->{mid}, 'edited message indexed'); + isnt($msg1->{blob}, $msg2->{blob}, 'blobs differ'); + my $eml2 = $es->smsg_eml($msg2); + like($eml2->body, qr/BEST MSG/, 'edited body in #2'); + unlike($eml2->body, qr/test message/, 'old body discarded in #2'); + my $eml1 = $es->smsg_eml($msg1); + like($eml1->body, qr/test message/, 'original body in #1'); + my $x1 = $es->over->get_xref3(1); + my $x2 = $es->over->get_xref3(2); + is(scalar(@$x1), 1, 'original only has one xref3'); + is(scalar(@$x2), 1, 'new message has one xref3'); + isnt($x1->[0], $x2->[0], 'xref3 differs'); +} + done_testing;