* [PATCH 0/3] avoid msgmap reopens in long-lived processes
@ 2020-07-14 2:14 10% Eric Wong
0 siblings, 0 replies; 4+ results
From: Eric Wong @ 2020-07-14 2:14 UTC (permalink / raw)
To: meta
As with commit 2a717d13f10fcdc69921d80cf94c47a694a175d4
("nntpd+imapd: detect replaced over.sqlite3"), this is
another step towards eliminating needless wakeups on
systems with inotify or kqueue.
To save memory, we'll also stop storing {filename} in Perl once
the SQLite DB is open, since we expect to have thousands of
inboxes soon.
Eric Wong (3):
over: unset sqlite_unicode attribute
nntpd+imapd: detect unlinked msgmap
over+msgmap: do not store filename after DBI->connect
lib/PublicInbox/Inbox.pm | 11 +++----
lib/PublicInbox/Msgmap.pm | 67 ++++++++++++++++++++------------------
lib/PublicInbox/Over.pm | 31 +++++++++++++-----
lib/PublicInbox/OverIdx.pm | 6 ++--
t/nntpd.t | 8 +++++
5 files changed, 74 insertions(+), 49 deletions(-)
^ permalink raw reply [relevance 10%]
* [PATCH] imap: improve IDLE handling at graceful shutdown
@ 2020-06-15 7:43 7% Eric Wong
0 siblings, 0 replies; 4+ results
From: Eric Wong @ 2020-06-15 7:43 UTC (permalink / raw)
To: meta
Since IMAP IDLE users aren't expected to issue any commands, we
can terminate their connections immediately on graceful
shutdown.
Furthermore, we need to drop the inotify FD from the epoll set
to avoid warnings during global destruction. Embarassingly,
this required fixing wacky test ordering from 2a717d13f10fcdc6
("nntpd+imapd: detect replaced over.sqlite3")
---
lib/PublicInbox/IMAP.pm | 4 ++++
lib/PublicInbox/InboxIdle.pm | 4 ++++
t/nntpd.t | 34 ++++++++++++++++------------------
3 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 9ae7c60e..d4ef6efe 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -1462,6 +1462,10 @@ sub cmd_starttls ($$) {
# for graceful shutdown in PublicInbox::Daemon:
sub busy {
my ($self, $now) = @_;
+ if (defined($self->{-idle_tag})) {
+ $self->write(\"* BYE server shutting down\r\n");
+ return; # not busy anymore
+ }
($self->{rbuf} || $self->{wbuf} || $self->not_idle_long($now));
}
diff --git a/lib/PublicInbox/InboxIdle.pm b/lib/PublicInbox/InboxIdle.pm
index d60d4f23..d0bb43c5 100644
--- a/lib/PublicInbox/InboxIdle.pm
+++ b/lib/PublicInbox/InboxIdle.pm
@@ -79,4 +79,8 @@ sub event_step {
warn "$self->{inot}->poll err: $@\n" if $@;
}
+# for graceful shutdown in PublicInbox::Daemon,
+# just ensure the FD gets closed ASAP and subscribers
+sub busy { 0 }
+
1;
diff --git a/t/nntpd.t b/t/nntpd.t
index d6042d18..b24720eb 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -316,23 +316,6 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000
my @xap = grep m!Search/Xapian!, @of;
is_deeply(\@xap, [], 'Xapian not loaded in nntpd');
}
- {
- setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1);
- syswrite($s, 'HDR List-id 1-');
- select(undef, undef, undef, 0.15);
- ok($td->kill, 'killed nntpd');
- select(undef, undef, undef, 0.15);
- syswrite($s, "\r\n");
- $buf = '';
- do {
- sysread($s, $buf, 4096, length($buf));
- } until ($buf =~ /\r\n\z/);
- my @r = split("\r\n", $buf);
- like($r[0], qr/^5\d\d /,
- 'got 5xx response for unoptimized HDR');
- is(scalar @r, 1, 'only one response line');
- }
-
# -compact requires Xapian
SKIP: {
require_mods('Search::Xapian', 2);
@@ -356,7 +339,22 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000
my @of = xqx([$lsof, '-p', $td->{pid}], undef, $noerr);
is(scalar(grep(/\(deleted\)/, @of)), 0, 'no deleted files');
};
-
+ {
+ setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1);
+ syswrite($s, 'HDR List-id 1-');
+ select(undef, undef, undef, 0.15);
+ ok($td->kill, 'killed nntpd');
+ select(undef, undef, undef, 0.15);
+ syswrite($s, "\r\n");
+ $buf = '';
+ do {
+ sysread($s, $buf, 4096, length($buf));
+ } until ($buf =~ /\r\n\z/);
+ my @r = split("\r\n", $buf);
+ like($r[0], qr/^5\d\d /,
+ 'got 5xx response for unoptimized HDR');
+ is(scalar @r, 1, 'only one response line');
+ }
$n = $s = undef;
$td->join;
is($?, 0, 'no error in exited process');
^ permalink raw reply related [relevance 7%]
* Re: [PATCH] nntpd+imapd: detect replaced over.sqlite3
2020-06-11 0:57 14% ` [PATCH] nntpd+imapd: detect replaced over.sqlite3 Eric Wong
@ 2020-06-11 1:54 11% ` Kyle Meyer
0 siblings, 0 replies; 4+ results
From: Kyle Meyer @ 2020-06-11 1:54 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
Eric Wong writes:
> Anyways, the following patch (which goes on top of the imapd
> stuff I posted yesterday) should fix it.
Nice, thank you. (Looking forward to trying out the imapd stuff :))
^ permalink raw reply [relevance 11%]
* [PATCH] nntpd+imapd: detect replaced over.sqlite3
@ 2020-06-11 0:57 14% ` Eric Wong
2020-06-11 1:54 11% ` Kyle Meyer
0 siblings, 1 reply; 4+ results
From: Eric Wong @ 2020-06-11 0:57 UTC (permalink / raw)
To: Kyle Meyer; +Cc: meta
Eric Wong <e@80x24.org> wrote:
> Will have to think about how -compact needs to interact with
> read-only daemons w.r.t. over.sqlite3, and whether moving
> over.sqlite3 can be avoided.
It probably can't be avoided for v1 or I would've done it.
And I've been thinking about making public-inbox-compact support
VACUUM and possibly other expensive optimizations, too.
Anyways, the following patch (which goes on top of the imapd
stuff I posted yesterday) should fix it.
-------8<-------
Subject: [PATCH] nntpd+imapd: detect replaced over.sqlite3
For v1 inboxes (and possibly v2 in the future, for VACUUM),
public-inbox-compact replaces over.sqlite3 with a new file.
This currently doesn't need an extra inotify watch descriptor
(or FD for kevent) at the moment, so it can coexist nicely for
systems w/o IO::KQueue or Linux::Inotify2.
---
lib/PublicInbox/Inbox.pm | 14 +++++++++++---
lib/PublicInbox/NNTP.pm | 4 +++-
lib/PublicInbox/NNTPD.pm | 12 ++++++++++--
lib/PublicInbox/Over.pm | 36 +++++++++++++++++++++++++++++-------
t/nntpd.t | 35 +++++++++++++++++++++++++++++++----
5 files changed, 84 insertions(+), 17 deletions(-)
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index b2b0b56fdd3..7d5e048363f 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 over)) {
+ foreach my $f (qw(mm search)) {
# we bump refcnt by assigning tmp, here:
my $tmp = $ibx->{$f} or next;
next if Devel::Peek::SvREFCNT($tmp) > 2;
@@ -45,9 +45,9 @@ sub cleanup_task () {
$again = 1 if $git->cleanup;
}
}
+ check_inodes($ibx);
if ($have_devel_peek) {
- $again ||= !!($ibx->{over} || $ibx->{mm} ||
- $ibx->{search});
+ $again ||= !!($ibx->{mm} || $ibx->{search});
}
$next->{"$ibx"} = $ibx if $again;
}
@@ -407,9 +407,17 @@ sub unsubscribe_unlock {
delete $self->{unlock_subs}->{$ident};
}
+sub check_inodes ($) {
+ my ($self) = @_;
+ for (qw(over)) { # TODO: search, mm
+ $self->{$_}->check_inodes if $self->{$_};
+ }
+}
+
# called by inotify
sub on_unlock {
my ($self) = @_;
+ check_inodes($self);
my $subs = $self->{unlock_subs} or return;
for (values %$subs) {
eval { $_->on_inbox_unlock($self) };
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index ac13c7df8ce..bffd773cf9e 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -334,7 +334,9 @@ sub cmd_newnews ($$$$;$$) {
sub cmd_group ($$) {
my ($self, $group) = @_;
my $no_such = '411 no such news group';
- my $ng = $self->{nntpd}->{groups}->{$group} or return $no_such;
+ my $nntpd = $self->{nntpd};
+ my $ng = $nntpd->{groups}->{$group} or return $no_such;
+ $nntpd->idler_start;
$self->{ng} = $ng;
my ($min, $max) = $ng->mm->minmax;
diff --git a/lib/PublicInbox/NNTPD.pm b/lib/PublicInbox/NNTPD.pm
index ed5cf7cc8c0..6b762d8923b 100644
--- a/lib/PublicInbox/NNTPD.pm
+++ b/lib/PublicInbox/NNTPD.pm
@@ -8,6 +8,7 @@ use strict;
use warnings;
use Sys::Hostname;
use PublicInbox::Config;
+use PublicInbox::InboxIdle;
sub new {
my ($class) = @_;
@@ -24,15 +25,17 @@ sub new {
err => \*STDERR,
out => \*STDOUT,
grouplist => [],
+ pi_config => $pi_config,
servername => $name,
greet => \"201 $name ready - post via email\r\n",
# accept_tls => { SSL_server => 1, ..., SSL_reuse_ctx => ... }
+ # idler => PublicInbox::InboxIdle
}, $class;
}
sub refresh_groups {
- my ($self, $pi_config) = @_;
- $pi_config //= PublicInbox::Config->new;
+ my ($self, $sig) = @_;
+ my $pi_config = $sig ? PublicInbox::Config->new : $self->{pi_config};
my $new = {};
my @list;
$pi_config->each_inbox(sub {
@@ -59,8 +62,13 @@ sub refresh_groups {
});
@list = sort { $a->{newsgroup} cmp $b->{newsgroup} } @list;
$self->{grouplist} = \@list;
+ $self->{pi_config} = $pi_config;
# this will destroy old groups that got deleted
%{$self->{groups}} = %$new;
}
+sub idler_start {
+ $_[0]->{idler} //= PublicInbox::InboxIdle->new($_[0]->{pi_config});
+}
+
1;
diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index e32104f0cf8..74c8fb86270 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -19,13 +19,23 @@ sub dbh_new {
if ($rw && !-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 => !$rw,
- sqlite_use_immediate_transaction => 1,
- });
+ my (@st, $st, $dbh);
+ my $tries = 0;
+ do {
+ @st = stat($f) or die "failed to stat $f: $!";
+ $st = pack('dd', $st[0], $st[1]); # 0: dev, 1: inode
+ $dbh = DBI->connect("dbi:SQLite:dbname=$f",'','', {
+ AutoCommit => 1,
+ RaiseError => 1,
+ PrintError => 0,
+ ReadOnly => !$rw,
+ sqlite_use_immediate_transaction => 1,
+ });
+ $self->{st} = $st;
+ @st = stat($f) or die "failed to stat $f: $!";
+ $st = pack('dd', $st[0], $st[1]);
+ } while ($st ne $self->{st} && $tries++ < 3);
+ warn "W: $f: .st_dev, .st_ino unstable\n" if $st ne $self->{st};
$dbh->{sqlite_unicode} = 1;
$dbh;
}
@@ -259,4 +269,16 @@ SELECT MAX(num) FROM over WHERE num > 0
($exists, $uidnext, $sth->fetchrow_array // 0);
}
+sub check_inodes {
+ my ($self) = @_;
+ if (my @st = stat($self->{filename})) { # 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));
+ } else {
+ warn "W: stat $self->{filename}: $!\n";
+ }
+}
+
1;
diff --git a/t/nntpd.t b/t/nntpd.t
index d2f31323115..d6042d18758 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -14,8 +14,11 @@ use Net::NNTP;
use Sys::Hostname;
# FIXME: make easier to test both versions
-my $version = $ENV{PI_TEST_VERSION} || 2;
+my $version = $ENV{PI_TEST_VERSION} || 1;
require_git('2.6') if $version == 2;
+my $lsof = which('lsof');
+my $fast_idle = eval { require Linux::Inotify2; 1 } //
+ eval { require IO::KQueue; 1 };
my ($tmpdir, $for_destroy) = tmpdir();
my $home = "$tmpdir/pi-home";
@@ -302,13 +305,13 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000
is($rdr, waitpid($rdr, 0), 'reader done');
is($? >> 8, 0, 'no errors');
}
+ my $noerr = { 2 => \(my $null) };
SKIP: {
if ($INC{'Search/Xapian.pm'} && ($ENV{TEST_RUN_MODE}//2)) {
skip 'Search/Xapian.pm pre-loaded (by t/run.perl?)', 1;
}
- my $lsof = which('lsof') or skip 'lsof missing', 1;
- my $rdr = { 2 => \(my $null) };
- my @of = xqx([$lsof, '-p', $td->{pid}], undef, $rdr);
+ $lsof or skip 'lsof missing', 1;
+ my @of = xqx([$lsof, '-p', $td->{pid}], undef, $noerr);
skip('lsof broken', 1) if (!scalar(@of) || $?);
my @xap = grep m!Search/Xapian!, @of;
is_deeply(\@xap, [], 'Xapian not loaded in nntpd');
@@ -330,6 +333,30 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000
is(scalar @r, 1, 'only one response line');
}
+ # -compact requires Xapian
+ SKIP: {
+ require_mods('Search::Xapian', 2);
+ which('xapian-compact') or skip 'xapian-compact missing', 2;
+ is(xsys(qw(git config), "--file=$home/.public-inbox/config",
+ "publicinbox.$group.indexlevel", 'medium'),
+ 0, 'upgraded indexlevel');
+ my $ex = eml_load('t/data/0001.patch');
+ is($n->article($ex->header('Message-ID')), undef,
+ 'article did not exist');
+ $im->add($ex);
+ $im->done;
+ ok(run_script([qw(-index --reindex -c), $ibx->{inboxdir}],
+ undef, $noerr), '-compacted');
+ select(undef, undef, undef, $fast_idle ? 0.1 : 2.1);
+ $art = $n->article($ex->header('Message-ID'));
+ ok($art, 'new article retrieved after compact');
+ $lsof or skip 'lsof missing', 1;
+ ($^O =~ /\A(?:linux)\z/) or
+ skip "lsof /(deleted)/ check untested on $^O", 1;
+ my @of = xqx([$lsof, '-p', $td->{pid}], undef, $noerr);
+ is(scalar(grep(/\(deleted\)/, @of)), 0, 'no deleted files');
+ };
+
$n = $s = undef;
$td->join;
is($?, 0, 'no error in exited process');
^ permalink raw reply related [relevance 14%]
Results 1-4 of 4 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-06-09 22:13 news.public-inbox.org misbehaving? Kyle Meyer
2020-06-09 23:24 ` Eric Wong
2020-06-11 0:57 14% ` [PATCH] nntpd+imapd: detect replaced over.sqlite3 Eric Wong
2020-06-11 1:54 11% ` Kyle Meyer
2020-06-15 7:43 7% [PATCH] imap: improve IDLE handling at graceful shutdown Eric Wong
2020-07-14 2:14 10% [PATCH 0/3] avoid msgmap reopens in long-lived processes Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
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).