* [PATCH 07/82] imap: delay InboxIdle start, support refresh
2020-06-10 7:03 7% [PATCH 00/82] public-inbox-imapd: read-only IMAP server Eric Wong
@ 2020-06-10 7:04 5% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2020-06-10 7:04 UTC (permalink / raw)
To: meta
InboxIdle should not be holding onto Inbox objects after the
Config object they came from expires, and Config objects may
expire on SIGHUP.
Old Inbox objects still persist due to IMAP clients holding onto
them, but that's a concern we'll deal with at another time, or
not at all, since all clients expire, eventually.
Regardless, stale inotify watch descriptors should not be left
hanging after SIGHUP refreshes.
---
lib/PublicInbox/IMAP.pm | 1 +
lib/PublicInbox/IMAPD.pm | 14 +++++----
lib/PublicInbox/InboxIdle.pm | 36 ++++++++++++++++++----
t/imapd.t | 58 ++++++++++++++++++++++++++++++++++--
4 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 4a43185c512..c8592dc0329 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -160,6 +160,7 @@ sub cmd_idle ($$) {
# IDLE seems allowed by dovecot w/o a mailbox selected *shrug*
my $ibx = $self->{ibx} or return "$tag BAD no mailbox selected\r\n";
$ibx->subscribe_unlock(fileno($self->{sock}), $self);
+ $self->{imapd}->idler_start;
$self->{-idle_tag} = $tag;
$self->{-idle_max} = $ibx->mm->max // 0;
"+ idling\r\n"
diff --git a/lib/PublicInbox/IMAPD.pm b/lib/PublicInbox/IMAPD.pm
index 1922c16046a..05aa30e42a1 100644
--- a/lib/PublicInbox/IMAPD.pm
+++ b/lib/PublicInbox/IMAPD.pm
@@ -16,18 +16,22 @@ sub new {
out => \*STDOUT,
grouplist => [],
# accept_tls => { SSL_server => 1, ..., SSL_reuse_ctx => ... }
+ # pi_config => PublicInbox::Config
# idler => PublicInbox::InboxIdle
}, $class;
}
sub refresh_groups {
my ($self) = @_;
- if (my $old_idler = delete $self->{idler}) {
- $old_idler->close; # PublicInbox::DS::close
- }
- my $pi_config = PublicInbox::Config->new;
- $self->{idler} = PublicInbox::InboxIdle->new($pi_config);
+ my $pi_config = $self->{pi_config} = PublicInbox::Config->new;
$self->SUPER::refresh_groups($pi_config);
+ if (my $idler = $self->{idler}) {
+ $idler->refresh($pi_config);
+ }
+}
+
+sub idler_start {
+ $_[0]->{idler} //= PublicInbox::InboxIdle->new($_[0]->{pi_config});
}
1;
diff --git a/lib/PublicInbox/InboxIdle.pm b/lib/PublicInbox/InboxIdle.pm
index 095a801c946..c19b8d186cd 100644
--- a/lib/PublicInbox/InboxIdle.pm
+++ b/lib/PublicInbox/InboxIdle.pm
@@ -4,7 +4,8 @@
package PublicInbox::InboxIdle;
use strict;
use base qw(PublicInbox::DS);
-use fields qw(pi_config inot);
+use fields qw(pi_config inot pathmap);
+use Cwd qw(abs_path);
use Symbol qw(gensym);
use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
my $IN_CLOSE = 0x08 | 0x10; # match Linux inotify
@@ -19,13 +20,35 @@ if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) {
require PublicInbox::In2Tie if $ino_cls;
sub in2_arm ($$) { # PublicInbox::Config::each_inbox callback
- my ($ibx, $inot) = @_;
- my $path = "$ibx->{inboxdir}/";
- $path .= $ibx->version >= 2 ? 'inbox.lock' : 'ssoma.lock';
- $inot->watch($path, $IN_CLOSE, sub { $ibx->on_unlock });
+ my ($ibx, $self) = @_;
+ my $dir = abs_path($ibx->{inboxdir});
+ if (!defined($dir)) {
+ warn "W: $ibx->{inboxdir} not watched: $!\n";
+ return;
+ }
+ my $inot = $self->{inot};
+ my $cur = $self->{pathmap}->{$dir} //= [];
+
+ # transfer old subscriptions to the current inbox, cancel the old watch
+ if (my $old_ibx = $cur->[0]) {
+ $ibx->{unlock_subs} and
+ die "BUG: $dir->{unlock_subs} should not exist";
+ $ibx->{unlock_subs} = $old_ibx->{unlock_subs};
+ $cur->[1]->cancel;
+ }
+ $cur->[0] = $ibx;
+
+ my $lock = "$dir/".($ibx->version >= 2 ? 'inbox.lock' : 'ssoma.lock');
+ $cur->[1] = $inot->watch($lock, $IN_CLOSE, sub { $ibx->on_unlock });
+
# TODO: detect deleted packs (and possibly other files)
}
+sub refresh {
+ my ($self, $pi_config) = @_;
+ $pi_config->each_inbox(\&in2_arm, $self);
+}
+
sub new {
my ($class, $pi_config) = @_;
my $self = fields::new($class);
@@ -42,7 +65,8 @@ sub new {
$inot = PublicInbox::FakeInotify->new;
}
$self->{inot} = $inot;
- $pi_config->each_inbox(\&in2_arm, $inot);
+ $self->{pathmap} = {}; # inboxdir => [ ibx, watch1, watch2, watch3...]
+ refresh($self, $pi_config);
$self;
}
diff --git a/t/imapd.t b/t/imapd.t
index 359c4c033b2..b0caa8f1779 100644
--- a/t/imapd.t
+++ b/t/imapd.t
@@ -6,6 +6,7 @@ use Test::More;
use Time::HiRes ();
use PublicInbox::TestCommon;
use PublicInbox::Config;
+use PublicInbox::Spawn qw(which);
require_mods(qw(DBD::SQLite Mail::IMAPClient Linux::Inotify2));
my $level = '-Lbasic';
SKIP: {
@@ -141,9 +142,12 @@ is_deeply([$mic->has_capability('COMPRESS')], ['DEFLATE'], 'deflate cap');
ok($mic->compress, 'compress enabled');
$compress_logout->($mic);
+my $have_inotify = eval { require Linux::Inotify2; 1 };
+
my $pi_config = PublicInbox::Config->new;
$pi_config->each_inbox(sub {
my ($ibx) = @_;
+ my $env = { ORIGINAL_RECIPIENT => $ibx->{-primary_address} };
my $name = $ibx->{name};
my $ng = $ibx->{newsgroup};
my $mic = Mail::IMAPClient->new(%mic_opt);
@@ -154,12 +158,62 @@ $pi_config->each_inbox(sub {
ok($mic->idle, "IDLE succeeds on $ng");
open(my $fh, '<', 't/data/message_embed.eml') or BAIL_OUT("open: $!");
- my $env = { ORIGINAL_RECIPIENT => $ibx->{-primary_address} };
run_script(['-mda', '--no-precheck'], $env, { 0 => $fh }) or
BAIL_OUT('-mda delivery');
my $t0 = Time::HiRes::time();
ok(my @res = $mic->idle_data(11), "IDLE succeeds on $ng");
- ok(grep(/\A\* [0-9] EXISTS\b/, @res), 'got EXISTS message');
+ is(grep(/\A\* [0-9] EXISTS\b/, @res), 1, 'got EXISTS message');
+ ok((Time::HiRes::time() - $t0) < 10, 'IDLE client notified');
+
+ my (@ino_info, $ino_fdinfo);
+ SKIP: {
+ skip 'no inotify support', 1 unless $have_inotify;
+ skip 'missing /proc/$PID/fd', 1 if !-d "/proc/$td->{pid}/fd";
+ my @ino = grep {
+ readlink($_) =~ /\binotify\b/
+ } glob("/proc/$td->{pid}/fd/*");
+ is(scalar(@ino), 1, 'only one inotify FD');
+ my $ino_fd = (split('/', $ino[0]))[-1];
+ $ino_fdinfo = "/proc/$td->{pid}/fdinfo/$ino_fd";
+ if (open my $fh, '<', $ino_fdinfo) {
+ local $/ = "\n";
+ @ino_info = grep(/^inotify wd:/, <$fh>);
+ ok(scalar(@ino_info), 'inotify has watches');
+ } else {
+ skip "$ino_fdinfo missing: $!", 1;
+ }
+ };
+
+ # ensure IDLE persists across HUP, w/o extra watches or FDs
+ $td->kill('HUP') or BAIL_OUT "failed to kill -imapd: $!";
+ SKIP: {
+ skip 'no inotify fdinfo (or support)', 2 if !@ino_info;
+ my (@tmp, %prev);
+ local $/ = "\n";
+ my $end = time + 5;
+ until (time > $end) {
+ select undef, undef, undef, 0.01;
+ open my $fh, '<', $ino_fdinfo or
+ BAIL_OUT "$ino_fdinfo: $!";
+ %prev = map { $_ => 1 } @ino_info;
+ @tmp = grep(/^inotify wd:/, <$fh>);
+ if (scalar(@tmp) == scalar(@ino_info)) {
+ delete @prev{@tmp};
+ last if scalar(keys(%prev)) == @ino_info;
+ }
+ }
+ is(scalar @tmp, scalar @ino_info,
+ 'old inotify watches replaced');
+ is(scalar keys %prev, scalar @ino_info,
+ 'no previous watches overlap');
+ };
+
+ open($fh, '<', 't/data/0001.patch') or BAIL_OUT("open: $!");
+ run_script(['-mda', '--no-precheck'], $env, { 0 => $fh }) or
+ BAIL_OUT('-mda delivery');
+ $t0 = Time::HiRes::time();
+ ok(@res = $mic->idle_data(11), "IDLE succeeds on $ng after HUP");
+ is(grep(/\A\* [0-9] EXISTS\b/, @res), 1, 'got EXISTS message');
ok((Time::HiRes::time() - $t0) < 10, 'IDLE client notified');
});
^ permalink raw reply related [relevance 5%]
* [PATCH 00/82] public-inbox-imapd: read-only IMAP server
@ 2020-06-10 7:03 7% Eric Wong
2020-06-10 7:04 5% ` [PATCH 07/82] imap: delay InboxIdle start, support refresh Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2020-06-10 7:03 UTC (permalink / raw)
To: meta
So I finally wrote my first IMAP server! And I'm actually
fairly satisfied with how it's turning out to support a bunch
of other performance + scalability work I've wanted to do.
Some previous notes here:
https://public-inbox.org/meta/20200609113442.GA16856@dcvr/
I finally seem to have gotten it to play nicely with mutt header
caching, so it's fit for public consumption :)
imaps://news.public-inbox.org/INBOX.comp.mail.public-inbox.meta.0
You can use any username+password, and AUTH=ANONYMOUS also
works if your client does that.
It doesn't support UTF-7 (mailbox names) or advertise UTF-8
in CAPABILITIES, yet; I still have RFCs to read :P
And there's a bunch of new things which could use some
testing from non-mutt/mbsync/offlineimap users.
Maybe you'll find some client-side bugs like I did :P
v1 reindexing also gets a little bit of parallelism :)
Anyways, I'll probably be porting some of the scalability
and slow-storage work to older parts of the code before
fiddling with more IMAP extensions.
Eric Wong (82):
doc: add some IMAP standards
nntpd: restrict allowed newsgroup names
preliminary imap server implementation
inboxidle: new class to detect inbox changes
imap: support IDLE
msgmap: split ->max into its own method
imap: delay InboxIdle start, support refresh
imap: implement STATUS command
imap: use Text::ParseWords::parse_line to handle quoted words
imap: support LIST command
t/imapd: support FakeInotify and KQNotify
imap: support fetch for BODYSTRUCTURE and BODY
eml: each_part: single part $idx is 1
imap: allow fetch of partial of BODY[...] and headers
imap: always include `resp-text' in responses
imap: split out unit tests and benchmarks
imap: fix multi-message partial header fetches
imap: simplify partial fetch structure
imap: support sequence number FETCH
imap: do not include ".PEEK" in responses
imap: support the CLOSE command
imap: speed up HEADER.FIELDS[.NOT] range fetches
git: async: flatten the inflight array
git: do our own read buffering for cat-file
imap: use git-cat-file asynchronously
git: idle rbuf for async
imap: support LSUB command
imap: FETCH: support comma-delimited ranges
add imapd compression test
testcommon: tcp_(server|connect): BAIL_OUT on failure
*deflate: drop invalid comment about rbuf
imap: fix pipelining with async git
git: cat_async: provide requested OID + "missing" on missing blobs
git: move async_cat reference to PublicInbox::Git
git: async: automatic retry on alternates change
imapclient: wrapper for Mail::IMAPClient
xt: add imapd-validate and imapd-mbsync-oimap
imap: support out-of-bounds ranges
xt/perf-imap-list: time refresh_inboxlist
imap: case-insensitive mailbox name comparisons
imap: break giant inboxes into sub-inboxes of 50K messages
imap: start introducing iterative config reloading
imap: require ".$UID_MIN-$UID_END" suffix
imapd: ensure LIST is sorted alphabetically, for now
imap: omit $UID_END from mailbox name, use index
t/config.t: always compare against git bool behavior
xt/*: show some tunable parameters
imap: STATUS and LIST are case-insensitive, too
imap: EXAMINE/STATUS: return correct counts
imap: avoid uninitialized warnings on incomplete commands
imap: start parsing out queries for SQLite and Xapian
imap: SEARCH: clamp results to the 50K UID range
imap: allow UID range search on timestamps
over: get_art: use dbh->prepare_cached
search: index byte size of a message for IMAP search
search: index UID for IMAP search, too
imap: remove dummies from sequence number FETCH
imap: compile UID FETCH to opcodes
imap: UID FETCH: optimize for smsg-only case
imap: UID FETCH: optimize (UID FLAGS) harder
imap: IDLE: avoid extraneous wakeups, keep-alive
imap: 30 minute auto-logout timer
imap: split ->logged_in attribute into a separate class
searchidx: v1 (re)-index uses git asynchronously
index: account for CRLF conversion when storing bytes
imap: rely on smsg->{bytes} for RFC822.SIZE
imap: UID FETCH requires at least one data item
imap: LIST shows "INBOX" in all caps
imap: support 8000 octet lines
imap: reinstate some message sequence number support
imap: cleanup ->{uid_base} usage
imap: FETCH: more granular CRLF conversion
imap: further speed up HEADER.FIELDS FETCH requests
imap: FETCH: try to make fake MSNs sequentially
imap: STATUS/EXAMINE: rely on SQLite overview
imap: UID SEARCH: support multiple ranges
imap: wire up Xapian search, msn SEARCH and multiple ranges
imap: misc cleanups and notes
imapd: don't bother sorting LIST output
imap: drop non-UID SEARCH for now
over: uid_range: remove LIMIT
imap: FETCH: proper MSN => UID mapping for requests
Documentation/public-inbox-imapd.pod | 91 ++
Documentation/standards.perl | 10 +
MANIFEST | 18 +
lib/PublicInbox/Config.pm | 18 +
lib/PublicInbox/Daemon.pm | 24 +-
lib/PublicInbox/DummyInbox.pm | 22 +
lib/PublicInbox/Eml.pm | 9 +-
lib/PublicInbox/FakeInotify.pm | 59 ++
lib/PublicInbox/Git.pm | 163 +--
lib/PublicInbox/GitAsyncCat.pm | 51 +
lib/PublicInbox/IMAP.pm | 1397 ++++++++++++++++++++++++++
lib/PublicInbox/IMAPClient.pm | 119 +++
lib/PublicInbox/IMAPD.pm | 114 +++
lib/PublicInbox/IMAPdeflate.pm | 126 +++
lib/PublicInbox/Import.pm | 2 +-
lib/PublicInbox/In2Tie.pm | 17 +
lib/PublicInbox/Inbox.pm | 33 +-
lib/PublicInbox/InboxIdle.pm | 79 ++
lib/PublicInbox/KQNotify.pm | 66 ++
lib/PublicInbox/Lock.pm | 7 +
lib/PublicInbox/MsgIter.pm | 2 +-
lib/PublicInbox/Msgmap.pm | 20 +-
lib/PublicInbox/NNTPD.pm | 12 +-
lib/PublicInbox/NNTPdeflate.pm | 1 -
lib/PublicInbox/Over.pm | 50 +-
lib/PublicInbox/Search.pm | 32 +-
lib/PublicInbox/SearchIdx.pm | 89 +-
lib/PublicInbox/SearchIdxShard.pm | 11 +-
lib/PublicInbox/Smsg.pm | 8 +-
lib/PublicInbox/TestCommon.pm | 7 +-
lib/PublicInbox/V2Writable.pm | 10 +-
script/public-inbox-imapd | 14 +
t/config.t | 15 +-
t/eml.t | 2 +-
t/git.t | 40 +-
t/imap.t | 133 +++
t/imapd-tls.t | 204 ++++
t/imapd.t | 398 ++++++++
t/import.t | 5 +-
t/inbox_idle.t | 72 ++
t/nntpd.t | 5 +-
t/over.t | 3 +
t/search.t | 19 +
xt/cmp-msgstr.t | 1 -
xt/cmp-msgview.t | 1 -
xt/eml_check_limits.t | 6 +-
xt/git_async_cmp.t | 2 +-
xt/imapd-mbsync-oimap.t | 132 +++
xt/imapd-validate.t | 177 ++++
xt/mem-msgview.t | 1 +
xt/msgtime_cmp.t | 1 -
xt/perf-msgview.t | 1 -
52 files changed, 3718 insertions(+), 181 deletions(-)
create mode 100644 Documentation/public-inbox-imapd.pod
create mode 100644 lib/PublicInbox/DummyInbox.pm
create mode 100644 lib/PublicInbox/FakeInotify.pm
create mode 100644 lib/PublicInbox/GitAsyncCat.pm
create mode 100644 lib/PublicInbox/IMAP.pm
create mode 100644 lib/PublicInbox/IMAPClient.pm
create mode 100644 lib/PublicInbox/IMAPD.pm
create mode 100644 lib/PublicInbox/IMAPdeflate.pm
create mode 100644 lib/PublicInbox/In2Tie.pm
create mode 100644 lib/PublicInbox/InboxIdle.pm
create mode 100644 lib/PublicInbox/KQNotify.pm
create mode 100644 script/public-inbox-imapd
create mode 100644 t/imap.t
create mode 100644 t/imapd-tls.t
create mode 100644 t/imapd.t
create mode 100644 t/inbox_idle.t
create mode 100644 xt/imapd-mbsync-oimap.t
create mode 100644 xt/imapd-validate.t
^ permalink raw reply [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-06-10 7:03 7% [PATCH 00/82] public-inbox-imapd: read-only IMAP server Eric Wong
2020-06-10 7:04 5% ` [PATCH 07/82] imap: delay InboxIdle start, support refresh 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).