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 878251F5B7 for ; Tue, 14 Jul 2020 02:14:33 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/3] over+msgmap: do not store filename after DBI->connect Date: Tue, 14 Jul 2020 02:14:32 +0000 Message-Id: <20200714021432.11024-4-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: SQLite already knows the filename internally, so avoid having it as a long-lived Perl SV to save some bytes when there's many inboxes and open DBs. --- lib/PublicInbox/Inbox.pm | 4 ++-- lib/PublicInbox/Msgmap.pm | 14 ++++++++------ lib/PublicInbox/Over.pm | 30 ++++++++++++++++++++++-------- lib/PublicInbox/OverIdx.pm | 2 +- t/nntpd.t | 2 +- 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm index 3d9754dc..267be4e3 100644 --- a/lib/PublicInbox/Inbox.pm +++ b/lib/PublicInbox/Inbox.pm @@ -204,9 +204,9 @@ sub search ($;$) { sub over ($) { my ($self) = @_; my $srch = search($self, 1) or return; - $self->{over} ||= eval { + $self->{over} //= eval { my $over = $srch->{over_ro}; - $over->dbh_new; # may fail + $over->connect; # may fail $over; } } diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm index e86fb854..38ec7858 100644 --- a/lib/PublicInbox/Msgmap.pm +++ b/lib/PublicInbox/Msgmap.pm @@ -229,9 +229,9 @@ sub mid_set { sub DESTROY { my ($self) = @_; - delete $self->{dbh}; - my $f = $self->{filename}; + my $dbh = $self->{dbh} or return; if (($self->{pid} // 0) == $$) { + my $f = $dbh->sqlite_db_filename; unlink $f or warn "failed to unlink $f: $!\n"; } } @@ -239,8 +239,9 @@ sub DESTROY { sub atfork_parent { my ($self) = @_; $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} = PublicInbox::Over::dbh_new($self, 1); + my $dbh = $self->{dbh} and die "tmp_clone dbh not prepared for parent"; + $self->{filename} = $dbh->sqlite_db_filename; + $dbh = $self->{dbh} = PublicInbox::Over::dbh_new($self, 1); $dbh->do('PRAGMA synchronous = OFF'); } @@ -249,9 +250,10 @@ sub atfork_prepare { $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"; + my $dbh = $self->{dbh} or die "temporary clone not open\n"; + # must clobber prepared statements - %$self = (filename => $self->{filename}, pid => $$); + %$self = (filename => $dbh->sqlite_db_filename, pid => $$); } sub skip_artnum { diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm index 5d285057..e3f26456 100644 --- a/lib/PublicInbox/Over.pm +++ b/lib/PublicInbox/Over.pm @@ -15,9 +15,13 @@ use constant DEFAULT_LIMIT => 1000; sub dbh_new { my ($self, $rw) = @_; - my $f = $self->{filename}; - if ($rw && !-f $f) { # SQLite defaults mode to 0644, we want 0666 - open my $fh, '+>>', $f or die "failed to open $f: $!"; + my $f = delete $self->{filename}; + if (!-f $f) { # SQLite defaults mode to 0644, we want 0666 + if ($rw) { + open my $fh, '+>>', $f or die "failed to open $f: $!"; + } else { + $self->{filename} = $f; # die on stat() below: + } } my (@st, $st, $dbh); my $tries = 0; @@ -44,9 +48,14 @@ sub new { bless { filename => $f }, $class; } -sub disconnect { $_[0]->{dbh} = undef } +sub disconnect { + my ($self) = @_; + if (my $dbh = delete $self->{dbh}) { + $self->{filename} = $dbh->sqlite_db_filename; + } +} -sub connect { $_[0]->{dbh} ||= $_[0]->dbh_new } +sub connect { $_[0]->{dbh} //= $_[0]->dbh_new } sub load_from_row ($;$) { my ($smsg, $cull) = @_; @@ -258,13 +267,18 @@ SELECT COUNT(num) FROM over WHERE num > ? AND num <= ? sub check_inodes { my ($self) = @_; - if (my @st = stat($self->{filename})) { # did st_dev, st_ino change? + my $dbh = $self->{dbh} or return; + my $f = $dbh->sqlite_db_filename; + if (my @st = stat($f)) { # did st_dev, st_ino change? my $st = pack('dd', $st[0], $st[1]); # don't actually reopen, just let {dbh} be recreated later - delete($self->{dbh}) if ($st ne ($self->{st} // $st)); + if ($st ne ($self->{st} // $st)) { + delete($self->{dbh}); + $self->{filename} = $f; + } } else { - warn "W: stat $self->{filename}: $!\n"; + warn "W: stat $f: $!\n"; } } diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm index 13aa2d74..ea8da723 100644 --- a/lib/PublicInbox/OverIdx.pm +++ b/lib/PublicInbox/OverIdx.pm @@ -431,7 +431,7 @@ sub rollback_lazy { sub disconnect { my ($self) = @_; die "in transaction" if $self->{txn}; - $self->{dbh} = undef; + $self->SUPER::disconnect; } sub create { diff --git a/t/nntpd.t b/t/nntpd.t index 954e6e75..aaf6661d 100644 --- a/t/nntpd.t +++ b/t/nntpd.t @@ -343,7 +343,7 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000 $im->add($ex); $im->done; { - my $f = $ibx->mm->{filename}; + my $f = $ibx->mm->{dbh}->sqlite_db_filename; my $tmp = "$tmpdir/tmp.sqlite3"; $ibx->mm->{dbh}->sqlite_backup_to_file($tmp); delete $ibx->{mm};