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,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 57A951F5B1 for ; Tue, 14 Jul 2020 02:14:33 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/3] nntpd+imapd: detect unlinked msgmap Date: Tue, 14 Jul 2020 02:14:31 +0000 Message-Id: <20200714021432.11024-3-e@yhbt.net> In-Reply-To: <20200714021432.11024-1-e@yhbt.net> References: <20200714021432.11024-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: While it's even less common to experience a replaced msgmap.sqlite3 file, BOFHs may do the darndest things. This is another step towards reducing the number of needless wakeups we need to do in long-lived read-only daemons. --- lib/PublicInbox/Inbox.pm | 7 ++--- lib/PublicInbox/Msgmap.pm | 59 ++++++++++++++++++++------------------- t/nntpd.t | 8 ++++++ 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm index 02186dac..3d9754dc 100644 --- a/lib/PublicInbox/Inbox.pm +++ b/lib/PublicInbox/Inbox.pm @@ -31,7 +31,7 @@ sub cleanup_task () { for my $ibx (values %$CLEANUP) { my $again; if ($have_devel_peek) { - foreach my $f (qw(mm search)) { + foreach my $f (qw(search)) { # we bump refcnt by assigning tmp, here: my $tmp = $ibx->{$f} or next; next if Devel::Peek::SvREFCNT($tmp) > 2; @@ -47,7 +47,7 @@ sub cleanup_task () { } check_inodes($ibx); if ($have_devel_peek) { - $again ||= !!($ibx->{mm} || $ibx->{search}); + $again ||= !!$ibx->{search}; } $next->{"$ibx"} = $ibx if $again; } @@ -182,7 +182,6 @@ sub mm { my ($self) = @_; $self->{mm} ||= eval { require PublicInbox::Msgmap; - _cleanup_later($self); my $dir = $self->{inboxdir}; if ($self->version >= 2) { PublicInbox::Msgmap->new_file("$dir/msgmap.sqlite3"); @@ -409,7 +408,7 @@ sub unsubscribe_unlock { sub check_inodes ($) { my ($self) = @_; - for (qw(over)) { # TODO: search, mm + for (qw(over mm)) { # TODO: search $self->{$_}->check_inodes if $self->{$_}; } } diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm index aa07e344..e86fb854 100644 --- a/lib/PublicInbox/Msgmap.pm +++ b/lib/PublicInbox/Msgmap.pm @@ -13,6 +13,7 @@ use warnings; use DBI; use DBD::SQLite; use File::Temp qw(tempfile); +use PublicInbox::Over; sub new { my ($class, $git_dir, $writable) = @_; @@ -24,29 +25,13 @@ sub new { new_file($class, "$d/msgmap.sqlite3", $writable); } -sub dbh_new { - my ($f, $writable) = @_; - if ($writable && !-f $f) { # SQLite defaults mode to 0644, we want 0666 - open my $fh, '+>>', $f or die "failed to open $f: $!"; - } - my $dbh = DBI->connect("dbi:SQLite:dbname=$f",'','', { - AutoCommit => 1, - RaiseError => 1, - PrintError => 0, - ReadOnly => !$writable, - sqlite_use_immediate_transaction => 1, - }); - $dbh; -} - sub new_file { - my ($class, $f, $writable) = @_; - return if !$writable && !-r $f; + my ($class, $f, $rw) = @_; + return if !$rw && !-r $f; - my $dbh = dbh_new($f, $writable); - my $self = bless { dbh => $dbh }, $class; - - if ($writable) { + my $self = bless { filename => $f }, $class; + my $dbh = $self->{dbh} = PublicInbox::Over::dbh_new($self, $rw); + if ($rw) { create_tables($dbh); # TRUNCATE reduces I/O compared to the default (DELETE) @@ -70,7 +55,6 @@ sub tmp_clone { my $tmp = ref($self)->new_file($fn, 1); $tmp->{dbh}->do('PRAGMA synchronous = OFF'); $tmp->{dbh}->do('PRAGMA journal_mode = MEMORY'); - $tmp->{tmp_name} = $fn; # SQLite won't work if unlinked, apparently $tmp->{pid} = $$; close $fh or die "failed to close $fn: $!"; $tmp; @@ -246,28 +230,28 @@ sub mid_set { sub DESTROY { my ($self) = @_; delete $self->{dbh}; - my $f = delete $self->{tmp_name}; - if (defined $f && $self->{pid} == $$) { + my $f = $self->{filename}; + if (($self->{pid} // 0) == $$) { unlink $f or warn "failed to unlink $f: $!\n"; } } sub atfork_parent { my ($self) = @_; - my $f = $self->{tmp_name} or die "not a temporary clone\n"; + $self->{pid} or die "not a temporary clone\n"; delete $self->{dbh} and die "tmp_clone dbh not prepared for parent"; - my $dbh = $self->{dbh} = dbh_new($f, 1); + my $dbh = $self->{dbh} = PublicInbox::Over::dbh_new($self, 1); $dbh->do('PRAGMA synchronous = OFF'); } sub atfork_prepare { my ($self) = @_; - my $f = $self->{tmp_name} or die "not a temporary clone\n"; + $self->{pid} or die "not a temporary clone\n"; $self->{pid} == $$ or die "BUG: atfork_prepare not called from $self->{pid}\n"; $self->{dbh} or die "temporary clone not open\n"; # must clobber prepared statements - %$self = (tmp_name => $f, pid => $$); + %$self = (filename => $self->{filename}, pid => $$); } sub skip_artnum { @@ -296,4 +280,23 @@ sub skip_artnum { } } +sub check_inodes { + my ($self) = @_; + # no filename if in-:memory: + my $f = $self->{dbh}->sqlite_db_filename // return; + if (my @st = stat($f)) { # did st_dev, st_ino change? + my $st = pack('dd', $st[0], $st[1]); + if ($st ne ($self->{st} // $st)) { + my $tmp = eval { ref($self)->new_file($f) }; + if ($@) { + warn "E: DBI->connect($f): $@\n"; + } else { + %$self = %$tmp; + } + } + } else { + warn "W: stat $f: $!\n"; + } +} + 1; diff --git a/t/nntpd.t b/t/nntpd.t index 28008ec1..954e6e75 100644 --- a/t/nntpd.t +++ b/t/nntpd.t @@ -14,6 +14,7 @@ use Net::NNTP; use Sys::Hostname; use POSIX qw(_exit); use Digest::SHA; +use_ok 'PublicInbox::Msgmap'; # FIXME: make easier to test both versions my $version = $ENV{PI_TEST_VERSION} || 1; @@ -341,6 +342,13 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000 'article did not exist'); $im->add($ex); $im->done; + { + my $f = $ibx->mm->{filename}; + my $tmp = "$tmpdir/tmp.sqlite3"; + $ibx->mm->{dbh}->sqlite_backup_to_file($tmp); + delete $ibx->{mm}; + rename($tmp, $f) or BAIL_OUT "rename($tmp, $f): $!"; + } ok(run_script([qw(-index --reindex -c), $ibx->{inboxdir}], undef, $noerr), '-compacted'); select(undef, undef, undef, $fast_idle ? 0.1 : 2.1);