nntpd errors retrieving group list

nntpd errors retrieving group list

[Konstantin Ryabitsev]

Hello:

I tried the nntpd daemon on x-lore, but I seem to be hitting something odd
while retrieving the group list. This is from syslog:

public-inbox-nntpd: Can't call method "minmax" on an undefined value at /usr/local/share/perl5/PublicInbox/NNTP.pm line 264.
public-inbox-nntpd: during long response[14] - 0.0 03772

This is in nntpd.out.log

[14] MODE READER - 0.000052
[14] LIST - 0.003624 pending
 deferred[14] aborted - 0.003772

All entries in the config file have a newsgroup= entry, so I don't have any
immediate insights why something would be "undefined" at that line.

Best regards,
-K

Re: nntpd errors retrieving group list

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Hello:
> 
> I tried the nntpd daemon on x-lore, but I seem to be hitting something odd
> while retrieving the group list. This is from syslog:
> 
> public-inbox-nntpd: Can't call method "minmax" on an undefined value at /usr/local/share/perl5/PublicInbox/NNTP.pm line 264.
> public-inbox-nntpd: during long response[14] - 0.0 03772
> 
> This is in nntpd.out.log
> 
> [14] MODE READER - 0.000052
> [14] LIST - 0.003624 pending
>  deferred[14] aborted - 0.003772
> 
> All entries in the config file have a newsgroup= entry, so I don't have any
> immediate insights why something would be "undefined" at that line.

Any chance you're out-of-FDs or permissions are wrong?

There's also an off chance a non-inbox (extindex) object is there.
Perhaps this debugging patch can shine a light on things:

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 9df47133..6f07db84 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -261,8 +261,11 @@ sub parse_time ($$;$) {
 
 sub group_line ($$) {
 	my ($self, $ibx) = @_;
-	my ($min, $max) = $ibx->mm->minmax;
-	more($self, "$ibx->{newsgroup} $max $min n");
+	eval {
+		my ($min, $max) = $ibx->mm->minmax;
+		more($self, "$ibx->{newsgroup} $max $min n");
+	};
+	warn "Error: $@ - $ibx $ibx->{newsgroup}\n" if $@;
 }
 
 sub newgroups_i {

Re: nntpd errors retrieving group list

[Konstantin Ryabitsev]

On Tue, Aug 24, 2021 at 08:11:15PM +0000, Eric Wong wrote:
> Any chance you're out-of-FDs or permissions are wrong?
> 
> There's also an off chance a non-inbox (extindex) object is there.
> Perhaps this debugging patch can shine a light on things:

Aha, it did help identify the problem. There was a leftover entry in PI_CONFIG
that didn't actually have a corresponding directory on disk any  more (due to
me messing around). The problem only manifested itself in the nntpd daemon.

Cleaning up the bogus entry in PI_CONFIG makes everything work just fine, but
you may want to catch this condition early, as it may happen with mirroring
when grokmirror purges repositories without cleanly removing config entries
(I'm working on handling these situations via another grokmirror hook).

Thanks for your help!

Best regards,
-K

[PATCH] imap+nntp: die loudly if ->mm or ->over disappear

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Cleaning up the bogus entry in PI_CONFIG makes everything work just fine, but
> you may want to catch this condition early, as it may happen with mirroring

Agreed, I think the following should catch most cases:

----------8<---------
Subject: [PATCH] imap+nntp: die loudly if ->mm or ->over disappear

While the WWW front-end can gracefully handle ->mm and ->over
disappearing (in most cases), IMAP+NNTP front-ends are completely
dependent on these and failed mysteriously when they go missing
after startup.

These will hopefully make issues like what Konstantin
encountered more obvious:

Link: https://public-inbox.org/meta/20210824204855.ejspej4z7r2rpu63@nitro.local/
---
 lib/PublicInbox/IMAP.pm  | 25 +++++++++---------
 lib/PublicInbox/Inbox.pm | 19 +++++++-------
 lib/PublicInbox/NNTP.pm  | 56 ++++++++++++++++++++--------------------
 3 files changed, 51 insertions(+), 49 deletions(-)

diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 9402aa41..37e07dae 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -195,14 +195,14 @@ sub cmd_capability ($$) {
 # but uo2m_hibernate can compact and deduplicate it
 sub uo2m_ary_new ($;$) {
 	my ($self, $exists) = @_;
-	my $base = $self->{uid_base};
-	my $uids = $self->{ibx}->over->uid_range($base + 1, $base + UID_SLICE);
+	my $ub = $self->{uid_base};
+	my $uids = $self->{ibx}->over(1)->uid_range($ub + 1, $ub + UID_SLICE);
 
 	# convert UIDs to offsets from {base}
 	my @tmp; # [$UID_OFFSET] => $MSN
 	my $msn = 0;
-	++$base;
-	$tmp[$_ - $base] = ++$msn for @$uids;
+	++$ub;
+	$tmp[$_ - $ub] = ++$msn for @$uids;
 	$$exists = $msn if $exists;
 	\@tmp;
 }
@@ -243,7 +243,7 @@ sub uo2m_extend ($$;$) {
 	# need to extend the current range:
 	my $base = $self->{uid_base};
 	++$beg;
-	my $uids = $self->{ibx}->over->uid_range($beg, $base + UID_SLICE);
+	my $uids = $self->{ibx}->over(1)->uid_range($beg, $base + UID_SLICE);
 	return $uo2m if !scalar(@$uids);
 	my @tmp; # [$UID_OFFSET] => $MSN
 	my $write_method = $_[2] // 'msg_more';
@@ -342,7 +342,7 @@ sub cmd_idle ($$) {
 	my $fd = fileno($sock);
 	$self->{-idle_tag} = $tag;
 	# only do inotify on most recent slice
-	if ($ibx->over->max < $uid_end) {
+	if ($ibx->over(1)->max < $uid_end) {
 		$ibx->subscribe_unlock($fd, $self);
 		$self->{imapd}->idler_start;
 	}
@@ -393,7 +393,7 @@ sub inbox_lookup ($$;$) {
 	my ($ibx, $exists, $uidmax, $uid_base) = (undef, 0, 0, 0);
 	$mailbox = lc $mailbox;
 	$ibx = $self->{imapd}->{mailboxes}->{$mailbox} or return;
-	my $over = $ibx->over;
+	my $over = $ibx->over(1);
 	if ($over != $ibx) { # not a dummy
 		$mailbox =~ /\.([0-9]+)\z/ or
 				die "BUG: unexpected dummy mailbox: $mailbox\n";
@@ -418,7 +418,8 @@ sub inbox_lookup ($$;$) {
 		# if "INBOX.foo.bar" is selected and "INBOX.foo.bar.0",
 		# check for new UID ranges (e.g. "INBOX.foo.bar.1")
 		if (my $z = $self->{imapd}->{mailboxes}->{"$mailbox.0"}) {
-			ensure_slices_exist($self->{imapd}, $z, $z->over->max);
+			ensure_slices_exist($self->{imapd}, $z,
+						$z->over(1)->max);
 		}
 	}
 	($ibx, $exists, $uidmax + 1, $uid_base);
@@ -722,7 +723,7 @@ sub range_step ($$) {
 		uid_clamp($self, \$beg, \$end);
 	} elsif ($range =~ /\A([0-9]+):\*\z/) {
 		$beg = $1 + 0;
-		$end = $self->{ibx}->over->max;
+		$end = $self->{ibx}->over(1)->max;
 		$end = $uid_end if $end > $uid_end;
 		$beg = $end if $beg > $end;
 		uid_clamp($self, \$beg, \$end);
@@ -740,7 +741,7 @@ sub range_step ($$) {
 sub refill_range ($$$) {
 	my ($self, $msgs, $range_info) = @_;
 	my ($beg, $end, $range_csv) = @$range_info;
-	if (scalar(@$msgs = @{$self->{ibx}->over->query_xover($beg, $end)})) {
+	if (scalar(@$msgs = @{$self->{ibx}->over(1)->query_xover($beg, $end)})){
 		$range_info->[0] = $msgs->[-1]->{num} + 1;
 		return;
 	}
@@ -781,7 +782,7 @@ sub fetch_smsg { # long_response
 sub refill_uids ($$$;$) {
 	my ($self, $uids, $range_info, $sql) = @_;
 	my ($beg, $end, $range_csv) = @$range_info;
-	my $over = $self->{ibx}->over;
+	my $over = $self->{ibx}->over(1);
 	while (1) {
 		if (scalar(@$uids = @{$over->uid_range($beg, $end, $sql)})) {
 			$range_info->[0] = $uids->[-1] + 1; # update $beg
@@ -1114,7 +1115,7 @@ sub parse_imap_query ($$) {
 	my ($self, $query) = @_;
 	my $q = PublicInbox::IMAPsearchqp::parse($self, $query);
 	if (ref($q)) {
-		my $max = $self->{ibx}->over->max;
+		my $max = $self->{ibx}->over(1)->max;
 		my $beg = 1;
 		uid_clamp($self, \$beg, \$max);
 		$q->{range_info} = [ $beg, $max ];
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index b94ffdb0..df140cac 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -8,6 +8,7 @@ use PublicInbox::Git;
 use PublicInbox::MID qw(mid2path);
 use PublicInbox::Eml;
 use List::Util qw(max);
+use Carp qw(croak);
 
 # Long-running "git-cat-file --batch" processes won't notice
 # unlinked packs, so we need to restart those processes occasionally.
@@ -168,8 +169,8 @@ sub max_git_epoch {
 }
 
 sub mm {
-	my ($self) = @_;
-	$self->{mm} ||= eval {
+	my ($self, $req) = @_;
+	$self->{mm} //= eval {
 		require PublicInbox::Msgmap;
 		my $dir = $self->{inboxdir};
 		if ($self->version >= 2) {
@@ -177,7 +178,7 @@ sub mm {
 		} else {
 			PublicInbox::Msgmap->new($dir);
 		}
-	};
+	} // ($req ? croak("E: $@") : undef);
 }
 
 sub search {
@@ -195,19 +196,19 @@ sub search {
 sub isrch { $_[0]->{isrch} // search($_[0]) }
 
 sub over {
-	$_[0]->{over} //= eval {
-		my $srch = $_[0]->{search} //= do {
-			_cleanup_later($_[0]);
+	my ($self, $req) = @_;
+	$self->{over} //= eval {
+		my $srch = $self->{search} //= do {
+			_cleanup_later($self);
 			require PublicInbox::Search;
-			PublicInbox::Search->new($_[0]);
+			PublicInbox::Search->new($self);
 		};
 		my $over = PublicInbox::Over->new("$srch->{xpfx}/over.sqlite3");
 		$over->dbh; # may fail
 		$over;
-	};
+	} // ($req ? croak("E: $@") : undef);
 }
 
-
 sub try_cat {
 	my ($path) = @_;
 	open(my $fh, '<', $path) or return '';
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 9df47133..13a68bb8 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -202,7 +202,7 @@ sub cmd_list ($;$$) {
 
 sub listgroup_range_i {
 	my ($self, $beg, $end) = @_;
-	my $r = $self->{ibx}->mm->msg_range($beg, $end, 'num');
+	my $r = $self->{ibx}->mm(1)->msg_range($beg, $end, 'num');
 	scalar(@$r) or return;
 	$self->msg_more(join('', map { "$_->[0]\r\n" } @$r));
 	1;
@@ -210,7 +210,7 @@ sub listgroup_range_i {
 
 sub listgroup_all_i {
 	my ($self, $num) = @_;
-	my $ary = $self->{ibx}->mm->ids_after($num);
+	my $ary = $self->{ibx}->mm(1)->ids_after($num);
 	scalar(@$ary) or return;
 	more($self, join("\r\n", @$ary));
 	1;
@@ -261,7 +261,7 @@ sub parse_time ($$;$) {
 
 sub group_line ($$) {
 	my ($self, $ibx) = @_;
-	my ($min, $max) = $ibx->mm->minmax;
+	my ($min, $max) = $ibx->mm(1)->minmax;
 	more($self, "$ibx->{newsgroup} $max $min n");
 }
 
@@ -367,7 +367,7 @@ sub cmd_group ($$) {
 	$nntpd->idler_start;
 
 	$self->{ibx} = $ibx;
-	my ($min, $max) = $ibx->mm->minmax;
+	my ($min, $max) = $ibx->mm(1)->minmax;
 	$self->{article} = $min;
 	my $est_size = $max - $min;
 	"211 $est_size $min $max $group";
@@ -381,7 +381,7 @@ sub article_adj ($$) {
 	defined $n or return '420 no current article has been selected';
 
 	$n += $off;
-	my $mid = $ibx->mm->mid_for($n);
+	my $mid = $ibx->mm(1)->mid_for($n);
 	unless ($mid) {
 		$n = $off > 0 ? 'next' : 'previous';
 		return "421 no $n article in this group";
@@ -418,9 +418,8 @@ sub xref_by_tc ($$$) {
 			$by_addr->{lc($_)} // ()
 		} (PublicInbox::Address::emails($smsg->{$f} // ''));
 		for my $ibx (@ibxs) {
-			my $ngname = $ibx->{newsgroup} // next;
-			next if defined $xref->{$ngname};
-			$xref->{$ngname} = eval { $ibx->mm->num_for($mid) };
+			$xref->{$ibx->{newsgroup}} //=
+						$ibx->mm(1)->num_for($mid);
 		}
 	}
 }
@@ -437,13 +436,15 @@ sub xref ($$$) {
 		$xref = { $cur_ng => $smsg->{num} };
 		my $mid = $smsg->{mid};
 		for my $ibx (values %{$nntpd->{pi_cfg}->{-by_newsgroup}}) {
-			next if defined($xref->{$ibx->{newsgroup}});
-			my $num = eval { $ibx->mm->num_for($mid) } // next;
-			$xref->{$ibx->{newsgroup}} = $num;
+			$xref->{$ibx->{newsgroup}} //=
+						 $ibx->mm(1)->num_for($mid);
 		}
 	}
 	my $ret = "$nntpd->{servername} $cur_ng:".delete($xref->{$cur_ng});
-	$ret .= " $_:$xref->{$_}" for (sort keys %$xref);
+	for my $ng (sort keys %$xref) {
+		my $num = $xref->{$ng} // next;
+		$ret .= " $ng:$num";
+	}
 	$ret;
 }
 
@@ -504,7 +505,7 @@ find_ibx:
 				return '412 no newsgroup has been selected';
 	}
 found:
-	my $smsg = $ibx->over->get_art($n) or return $err;
+	my $smsg = $ibx->over(1)->get_art($n) or return $err;
 	$smsg->{-ibx} = $ibx;
 	if ($code == 223) { # STAT
 		set_art($self, $n);
@@ -618,7 +619,7 @@ sub get_range ($$) {
 	my $ibx = $self->{ibx} or return '412 no news group has been selected';
 	defined $range or return '420 No article(s) selected';
 	my ($beg, $end);
-	my ($min, $max) = $ibx->mm->minmax;
+	my ($min, $max) = $ibx->mm(1)->minmax;
 	if ($range =~ /\A([0-9]+)\z/) {
 		$beg = $end = $1;
 	} elsif ($range =~ /\A([0-9]+)-\z/) {
@@ -688,7 +689,7 @@ sub long_response ($$;@) {
 
 sub hdr_msgid_range_i {
 	my ($self, $beg, $end) = @_;
-	my $r = $self->{ibx}->mm->msg_range($beg, $end);
+	my $r = $self->{ibx}->mm(1)->msg_range($beg, $end);
 	@$r or return;
 	$self->msg_more(join('', map { "$_->[0] <$_->[1]>\r\n" } @$r));
 	1;
@@ -714,7 +715,7 @@ sub mid_lookup ($$) {
 	my ($self, $mid) = @_;
 	my $cur_ibx = $self->{ibx};
 	if ($cur_ibx) {
-		my $n = $cur_ibx->mm->num_for($mid);
+		my $n = $cur_ibx->mm(1)->num_for($mid);
 		return ($cur_ibx, $n) if defined $n;
 	}
 	my $pi_cfg = $self->{nntpd}->{pi_cfg};
@@ -744,7 +745,7 @@ EOF
 	} else { # slow path for non-ALL users
 		for my $ibx (values %{$pi_cfg->{-by_newsgroup}}) {
 			next if defined $cur_ibx && $ibx eq $cur_ibx;
-			my $n = $ibx->mm->num_for($mid);
+			my $n = $ibx->mm(1)->num_for($mid);
 			return ($ibx, $n) if defined $n;
 		}
 	}
@@ -754,7 +755,7 @@ EOF
 sub xref_range_i {
 	my ($self, $beg, $end) = @_;
 	my $ibx = $self->{ibx};
-	my $msgs = $ibx->over->query_xover($$beg, $end);
+	my $msgs = $ibx->over(1)->query_xover($$beg, $end);
 	scalar(@$msgs) or return;
 	$$beg = $msgs->[-1]->{num} + 1;
 	$self->msg_more(join('', map {
@@ -770,7 +771,7 @@ sub hdr_xref ($$$) { # optimize XHDR Xref [range] for rtin
 		my $mid = $1;
 		my ($ibx, $n) = mid_lookup($self, $mid);
 		return r430 unless $n;
-		my $smsg = $ibx->over->get_art($n) or return;
+		my $smsg = $ibx->over(1)->get_art($n) or return;
 		hdr_mid_response($self, $xhdr, $ibx, $n, $range,
 				xref($self, $ibx, $smsg));
 	} else { # numeric range
@@ -783,16 +784,15 @@ sub hdr_xref ($$$) { # optimize XHDR Xref [range] for rtin
 }
 
 sub over_header_for {
-	my ($over, $num, $field) = @_;
-	my $smsg = $over->get_art($num) or return;
+	my ($ibx, $num, $field) = @_;
+	my $smsg = $ibx->over(1)->get_art($num) or return;
 	return PublicInbox::Smsg::date($smsg) if $field eq 'date';
 	$smsg->{$field};
 }
 
 sub smsg_range_i {
 	my ($self, $beg, $end, $field) = @_;
-	my $over = $self->{ibx}->over;
-	my $msgs = $over->query_xover($$beg, $end);
+	my $msgs = $self->{ibx}->over(1)->query_xover($$beg, $end);
 	scalar(@$msgs) or return;
 	my $tmp = '';
 
@@ -816,7 +816,7 @@ sub hdr_smsg ($$$$) {
 	if (defined $range && $range =~ $ONE_MSGID) {
 		my ($ibx, $n) = mid_lookup($self, $1);
 		return r430 unless defined $n;
-		my $v = over_header_for($ibx->over, $n, $field);
+		my $v = over_header_for($ibx, $n, $field);
 		hdr_mid_response($self, $xhdr, $ibx, $n, $range, $v);
 	} else { # numeric range
 		$range = $self->{article} unless defined $range;
@@ -885,7 +885,7 @@ sub hdr_mid_response ($$$$$$) {
 
 sub xrover_i {
 	my ($self, $beg, $end) = @_;
-	my $h = over_header_for($self->{ibx}->over, $$beg, 'references');
+	my $h = over_header_for($self->{ibx}, $$beg, 'references');
 	more($self, "$$beg $h") if defined($h);
 	$$beg++ < $end;
 }
@@ -925,7 +925,7 @@ sub cmd_over ($;$) {
 	if ($range && $range =~ $ONE_MSGID) {
 		my ($ibx, $n) = mid_lookup($self, $1);
 		defined $n or return r430;
-		my $smsg = $ibx->over->get_art($n) or return r430;
+		my $smsg = $ibx->over(1)->get_art($n) or return r430;
 		more($self, '224 Overview information follows (multi-line)');
 
 		# Only set article number column if it's the current group
@@ -946,7 +946,7 @@ sub cmd_over ($;$) {
 sub xover_i {
 	my ($self, $beg, $end) = @_;
 	my $ibx = $self->{ibx};
-	my $msgs = $ibx->over->query_xover($$beg, $end);
+	my $msgs = $ibx->over(1)->query_xover($$beg, $end);
 	my $nr = scalar @$msgs or return;
 
 	# OVERVIEW.FMT
@@ -1014,7 +1014,7 @@ sub cmd_xpath ($$) {
 		}
 	} else { # slow path, no point in using long_response
 		for my $ibx (values %$groups) {
-			my $n = $ibx->mm->num_for($mid) // next;
+			my $n = $ibx->mm(1)->num_for($mid) // next;
 			push @paths, "$ibx->{newsgroup}/$n";
 		}
 	}