user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH] watch: don't burn CPU on IDLE failures
@ 2020-07-04 21:33 Eric Wong
  0 siblings, 0 replies; only message in thread
From: Eric Wong @ 2020-07-04 21:33 UTC (permalink / raw)
  To: meta

Network connections fail and need to be detected sooner rather
than later during IDLE to avoid backtrace floods.  In case the
IDLE process dies completely, don't respawn right away, either,
to avoid entering a respawn loop.

There's also a typo fix :P
---
 lib/PublicInbox/WatchMaildir.pm | 23 ++++++++++++++++++-----
 t/imapd.t                       | 29 +++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 23b2e9f11..7547f6e47 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -497,7 +497,8 @@ sub imap_idle_once ($$$$) {
 	}
 	$self->{idle_mic} = $mic; # for ->quit
 	my @res;
-	until ($self->{quit} || grep(/^\* [0-9]+ EXISTS/, @res) || $i <= 0) {
+	until ($self->{quit} || !$mic->IsConnected ||
+			grep(/^\* [0-9]+ EXISTS/, @res) || $i <= 0) {
 		@res = $mic->idle_data($i);
 		$i = $end - now();
 	}
@@ -520,8 +521,13 @@ sub watch_imap_idle_1 ($$$) {
 	local $0 = $uri->mailbox." $sec";
 	until ($self->{quit}) {
 		$mic //= PublicInbox::IMAPClient->new(%$mic_arg);
-		my $err = imap_fetch_all($self, $mic, $url);
-		$err //= imap_idle_once($self, $mic, $intvl, $url);
+		my $err;
+		if ($mic && $mic->IsConnected) {
+			$err = imap_fetch_all($self, $mic, $url);
+			$err //= imap_idle_once($self, $mic, $intvl, $url);
+		} else {
+			$err = "not connected: $!";
+		}
 		if ($err && !$self->{quit}) {
 			warn $err, "\n";
 			$mic = undef;
@@ -545,6 +551,13 @@ sub watch_atfork_parent ($) {
 	_done_for_now($self);
 }
 
+sub imap_idle_requeue ($) { # DS::add_timer callback
+	my ($self, $url_intvl) = @{$_[0]};
+	return if $self->{quit};
+	push @{$self->{idle_todo}}, $url_intvl;
+	event_step($self);
+}
+
 sub imap_idle_reap { # PublicInbox::DS::dwaitpid callback
 	my ($self, $pid) = @_;
 	my $url_intvl = delete $self->{idle_pids}->{$pid} or
@@ -553,8 +566,8 @@ sub imap_idle_reap { # PublicInbox::DS::dwaitpid callback
 	my ($url, $intvl) = @$url_intvl;
 	return if $self->{quit};
 	warn "W: PID=$pid on $url died: \$?=$?\n" if $?;
-	push @{$self->{idle_todo}}, $url_intvl;
-	PubicInbox::DS::requeue($self); # call ->event_step to respawn
+	PublicInbox::DS::add_timer(60,
+				\&imap_idle_requeue, [ $self, $url_intvl ]);
 }
 
 sub imap_idle_fork ($$) {
diff --git a/t/imapd.t b/t/imapd.t
index cf327e9fb..1ac6a4ab6 100644
--- a/t/imapd.t
+++ b/t/imapd.t
@@ -468,7 +468,7 @@ SKIP: {
 	my $obj = bless \$cb, 'PublicInbox::TestCommon::InboxWakeup';
 	$cfg->each_inbox(sub { $_[0]->subscribe_unlock('ident', $obj) });
 	my $watcherr = "$tmpdir/watcherr";
-	open my $err_wr, '>', $watcherr or BAIL_OUT $!;
+	open my $err_wr, '>>', $watcherr or BAIL_OUT $!;
 	open my $err, '<', $watcherr or BAIL_OUT $!;
 	my $w = start_script(['-watch'], undef, { 2 => $err_wr });
 
@@ -512,11 +512,36 @@ SKIP: {
 	seek($err, 0, 0);
 	my @err = grep(!/^I:/, <$err>);
 	is(@err, 0, 'no warnings/errors from -watch'.join(' ', @err));
+
+	if ($ENV{TEST_KILL_IMAPD}) { # not sure how reliable this test can be
+		xsys(qw(git config), "--file=$home/.public-inbox/config",
+			qw(--unset imap.PollInterval)) == 0
+			or BAIL_OUT "git config $?";
+		truncate($err_wr, 0) or BAIL_OUT $!;
+		my @t0 = times;
+		$w = start_script(['-watch'], undef, { 2 => $err_wr });
+		seek($err, 0, 0);
+		tick until (grep(/I: \S+ idling/, <$err>));
+		diag 'killing imapd, waiting for CPU spins';
+		my $delay = 0.11;
+		$td->kill(9);
+		tick $delay;
+		$w->kill;
+		$w->join;
+		is($?, 0, 'no error in exited -watch process');
+		my @t1 = times;
+		my $c = $t1[2] + $t1[3] - $t0[2] - $t0[3];
+		my $thresh = (0.9 * $delay);
+		diag "c=$c, threshold=$thresh";
+		ok($c < $thresh, 'did not burn much CPU');
+		is_deeply([grep(/ line \d+$/m, <$err>)], [],
+				'no backtraces from errors');
+	}
 }
 
 $td->kill;
 $td->join;
-is($?, 0, 'no error in exited process');
+is($?, 0, 'no error in exited process') if !$ENV{TEST_KILL_IMAPD};
 open my $fh, '<', $err or BAIL_OUT("open $err failed: $!");
 my $eout = do { local $/; <$fh> };
 unlike($eout, qr/wide/i, 'no Wide character warnings');

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-07-04 21:33 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-04 21:33 [PATCH] watch: don't burn CPU on IDLE failures Eric Wong

Code repositories for project(s) associated with this 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).