* Re: httpd 502s [was: trying to figure out 100% CPU usage in nntpd...]
2019-09-12 11:37 0% ` Konstantin Ryabitsev
@ 2019-09-13 3:12 0% ` Eric Wong
0 siblings, 0 replies; 5+ results
From: Eric Wong @ 2019-09-13 3:12 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Thu, Sep 12, 2019 at 08:35:03AM +0000, Eric Wong wrote:
> > Eric Wong <e@80x24.org> wrote:
> > > One more thing, are you running any extra middlewares in the
> > > .psgi file? Thanks.
>
> No, it's just vanilla what comes with the source.
OK, and Perl 5.16.3 from CentOS 7? (4:5.16.3-294.el7_6 RPM)
> > That's probably not it, I suspected the non-fileno path was
> > being hit, but I just tested a debug change on top of
> > b7cfd5aeff4b6b316b61b327af9c144776d77225 (branch: "unlink")
> > ("tmpfile: support O_APPEND and use it in DS::tmpio")
> > to fake the presence of a middleware wrapping psgi.input.
>
> I sent you a dump of lsof -p of all 4 processes after about 20 minutes
> of running. For another data point, the daemon was running in
> SELinux-permissive mode, to make sure unlinks aren't failing because of
> any permission errors.
It looks like there's a Perl reference leak (cycle) of some
sort holding on to FDs, since you have lots of input files and
pipes, yet only one established IPv4 connection. And the inodes
encoded into the filenames don't point to the connected socket,
even....
However, I'm not able to reproduce it on my CentOS 7 VM which
has nginx 1.12.2. I don't think nginx is a factor in this
since public-inbox-httpd is clearly not holding TCP sockets
open, even.
Not at all familiar with SELinux, but I'm just using the
defaults CentOS comes with and running both nginx +
public-inbox-httpd as a regular user.
That "if (0..." GitHTTPBackend patch definitely isn't needed
for testing anymore and only makes FD exhaustion happen sooner.
> Let me know if you would like any further info.
If there's a reference leak somewhere, this could also be part
of the high memory use you showed us a few months ago. Dunno
if you had many FDs back then.
I could see about adding explicit close() calls in a few
places, but that would make a corresponding memory leak
harder-to-notice, even.
I pushed out two patches to the "unlink" branch which may be
able to reproduce the issue on your end (I see nothing out of
the ordinary on my slow CentOS 7 VM or Debian machines/VMs)
* [PATCH] t/httpd-corner: check for leaking FDs and pipes
* [RFC] t/git-http-backend: add MANY_CLONE test
# no pipes should be present in -httpd with -W0
prove -lv t/httpd-corner.t
# unrelated note: there's 4 pipes from -W1 (the default),
# but I think 2 can be closed, actually...
GIANT_GIT_DIR=/path/to/git.git MANY_CLONE=1 prove -lv t/git-http-backend.t
If those updated test cases can't reproduce the problem,
can you reproduce this on erol or any other machines?
perhaps with a different Perl?
Thanks.
^ permalink raw reply [relevance 0%]
* Re: httpd 502s [was: trying to figure out 100% CPU usage in nntpd...]
2019-09-12 8:35 6% ` Eric Wong
@ 2019-09-12 11:37 0% ` Konstantin Ryabitsev
2019-09-13 3:12 0% ` Eric Wong
0 siblings, 1 reply; 5+ results
From: Konstantin Ryabitsev @ 2019-09-12 11:37 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
On Thu, Sep 12, 2019 at 08:35:03AM +0000, Eric Wong wrote:
> Eric Wong <e@80x24.org> wrote:
> > One more thing, are you running any extra middlewares in the
> > .psgi file? Thanks.
No, it's just vanilla what comes with the source.
> That's probably not it, I suspected the non-fileno path was
> being hit, but I just tested a debug change on top of
> b7cfd5aeff4b6b316b61b327af9c144776d77225 (branch: "unlink")
> ("tmpfile: support O_APPEND and use it in DS::tmpio")
> to fake the presence of a middleware wrapping psgi.input.
I sent you a dump of lsof -p of all 4 processes after about 20 minutes
of running. For another data point, the daemon was running in
SELinux-permissive mode, to make sure unlinks aren't failing because of
any permission errors.
Let me know if you would like any further info.
Best,
-K
^ permalink raw reply [relevance 0%]
* Re: httpd 502s [was: trying to figure out 100% CPU usage in nntpd...]
@ 2019-09-12 8:35 6% ` Eric Wong
2019-09-12 11:37 0% ` Konstantin Ryabitsev
0 siblings, 1 reply; 5+ results
From: Eric Wong @ 2019-09-12 8:35 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
Eric Wong <e@80x24.org> wrote:
> One more thing, are you running any extra middlewares in the
> .psgi file? Thanks.
That's probably not it, I suspected the non-fileno path was
being hit, but I just tested a debug change on top of
b7cfd5aeff4b6b316b61b327af9c144776d77225 (branch: "unlink")
("tmpfile: support O_APPEND and use it in DS::tmpio")
to fake the presence of a middleware wrapping psgi.input.
I've also pushed out an "unlink-fix" branch on top of
f4f0a3be0864721d90f9557ffe1c513b0289a74b which only
features one bugfix which wouldn't affect you, as well
as the tmpfile change to give more meanigful names:
solvergit: ensure Qspawn usage doesn't unref update-index stdin
tmpfile: give temporary files meaningful names
Note: if reusing existing working tree, be sure to rerun
"perl Makefile.PL" since new .pm files won't get picked up,
otherwise.
# force creation of extra temporary file for psgi.input
# in case some middleware overrides the psgi.input we
# set in HTTP.pm:
--------8<------
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index a8337035..2eaf221e 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -216,7 +216,7 @@ sub input_prepare {
my $input = $env->{'psgi.input'};
my $fd = eval { fileno($input) };
- if (defined $fd && $fd >= 0) {
+ if (0 && defined $fd && $fd >= 0) {
return { 0 => $fd };
}
my $id = "git-http.input.$env->{REMOTE_HOST}:$env->{REMOTE_PORT}";
^ permalink raw reply related [relevance 6%]
* [PATCH 2/2] tmpfile: support O_APPEND and use it in DS::tmpio
2019-09-12 8:34 6% [PATCH 0/2] tmpfile: new class to manage temporary files Eric Wong
@ 2019-09-12 8:34 7% ` Eric Wong
0 siblings, 0 replies; 5+ results
From: Eric Wong @ 2019-09-12 8:34 UTC (permalink / raw)
To: meta
Might as well share some code for temporary file creation
---
lib/PublicInbox/DS.pm | 14 ++++----------
lib/PublicInbox/Tmpfile.pm | 8 ++++----
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 1e51dc41..30a9641a 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -26,6 +26,7 @@ use warnings;
use 5.010_001;
use PublicInbox::Syscall qw(:epoll);
+use PublicInbox::Tmpfile;
use fields ('sock', # underlying socket
'rbuf', # scalarref, usually undef
@@ -33,7 +34,7 @@ use fields ('sock', # underlying socket
'wbuf_off', # offset into first element of wbuf to start writing at
);
-use Errno qw(EAGAIN EINVAL EEXIST);
+use Errno qw(EAGAIN EINVAL);
use Carp qw(croak confess carp);
require File::Spec;
@@ -488,15 +489,8 @@ sub drop {
# PerlIO::mmap or PerlIO::scalar if needed
sub tmpio ($$$) {
my ($self, $bref, $off) = @_;
- my $fh; # open(my $fh, '+>>', undef) doesn't set O_APPEND
- do {
- my $fn = File::Spec->tmpdir . '/wbuf-' . rand;
- if (sysopen($fh, $fn, O_RDWR|O_CREAT|O_EXCL|O_APPEND, 0600)) { # likely
- unlink($fn) or return drop($self, "unlink($fn) $!");
- } elsif ($! != EEXIST) { # EMFILE/ENFILE/ENOSPC/ENOMEM
- return drop($self, "open: $!");
- }
- } until (defined $fh);
+ my $fh = tmpfile('wbuf', $self->{sock}, 1) or
+ return drop($self, "tmpfile $!");
$fh->autoflush(1);
my $len = bytes::length($$bref) - $off;
$fh->write($$bref, $len, $off) or return drop($self, "write ($len): $!");
diff --git a/lib/PublicInbox/Tmpfile.pm b/lib/PublicInbox/Tmpfile.pm
index 7fda100e..28e87f88 100644
--- a/lib/PublicInbox/Tmpfile.pm
+++ b/lib/PublicInbox/Tmpfile.pm
@@ -12,10 +12,9 @@ require File::Spec;
# use tmpfile instead of open(..., '+>', undef) so we can get an
# unlinked filename which makes sense when viewed with lsof
# (at least on Linux)
-# TODO: O_APPEND support (this is the reason I'm not using File::Temp)
# And if we ever stop caring to have debuggable filenames, O_TMPFILE :)
-sub tmpfile ($;$) {
- my ($id, $sock) = @_;
+sub tmpfile ($;$$) {
+ my ($id, $sock, $append) = @_;
if (defined $sock) {
# add the socket inode number so we can figure out which
# socket it belongs to
@@ -25,10 +24,11 @@ sub tmpfile ($;$) {
$id =~ tr!/!^!;
my $fl = O_RDWR | O_CREAT | O_EXCL;
+ $fl |= O_APPEND if $append;
do {
my $fn = File::Spec->tmpdir . "/$id-".time.'-'.rand;
if (sysopen(my $fh, $fn, $fl, 0600)) { # likely
- unlink($fn) or die "unlink($fn): $!"; # FS broken
+ unlink($fn) or warn "unlink($fn): $!"; # FS broken
return $fh; # success
}
} while ($! == EEXIST);
^ permalink raw reply related [relevance 7%]
* [PATCH 0/2] tmpfile: new class to manage temporary files
@ 2019-09-12 8:34 6% Eric Wong
2019-09-12 8:34 7% ` [PATCH 2/2] tmpfile: support O_APPEND and use it in DS::tmpio Eric Wong
0 siblings, 1 reply; 5+ results
From: Eric Wong @ 2019-09-12 8:34 UTC (permalink / raw)
To: meta
In order to improve debugging experience when looking at lsof(8)
output, create temporary files with useful names instead of
relying on open(..., "+>", undef).
We're going this route (instead of using File::Temp) since I
need that retry logic to create temporary files with O_APPEND,
anyways. And being able to encode the inode number of the
associated socket is nice.
Eric Wong (2):
tmpfile: give temporary files meaningful names
tmpfile: support O_APPEND and use it in DS::tmpio
MANIFEST | 1 +
lib/PublicInbox/DS.pm | 14 ++++--------
lib/PublicInbox/Git.pm | 4 +++-
lib/PublicInbox/GitHTTPBackend.pm | 4 +++-
lib/PublicInbox/HTTP.pm | 10 ++++----
lib/PublicInbox/SolverGit.pm | 3 ++-
lib/PublicInbox/Tmpfile.pm | 38 +++++++++++++++++++++++++++++++
lib/PublicInbox/ViewVCS.pm | 3 ++-
8 files changed, 59 insertions(+), 18 deletions(-)
create mode 100644 lib/PublicInbox/Tmpfile.pm
^ permalink raw reply [relevance 6%]
Results 1-5 of 5 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2019-09-09 10:05 trying to figure out 100% CPU usage in nntpd Konstantin Ryabitsev
2019-09-09 17:53 ` Eric Wong
2019-09-10 8:38 ` Konstantin Ryabitsev
2019-09-10 18:12 ` Eric Wong
2019-09-11 2:22 ` httpd 502s [was: trying to figure out 100% CPU usage in nntpd...] Eric Wong
2019-09-11 10:24 ` Konstantin Ryabitsev
2019-09-11 17:12 ` Eric Wong
2019-09-11 17:36 ` Konstantin Ryabitsev
2019-09-12 0:05 ` Eric Wong
2019-09-12 2:49 ` Eric Wong
2019-09-12 8:35 6% ` Eric Wong
2019-09-12 11:37 0% ` Konstantin Ryabitsev
2019-09-13 3:12 0% ` Eric Wong
2019-09-12 8:34 6% [PATCH 0/2] tmpfile: new class to manage temporary files Eric Wong
2019-09-12 8:34 7% ` [PATCH 2/2] tmpfile: support O_APPEND and use it in DS::tmpio 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).