* Re: [PATCH 0/2] leak workarounds for Perl 5.16 on CentOS/RHEL 7
2019-09-26 12:36 0% ` Konstantin Ryabitsev
@ 2019-09-26 21:16 0% ` Eric Wong
0 siblings, 0 replies; 5+ results
From: Eric Wong @ 2019-09-26 21:16 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Thu, Sep 26, 2019 at 01:50:36AM +0000, Eric Wong wrote:
> > After many hours of reviewing our code in PublicInbox::Qspawn,
> > PublicInbox::GitHTTPBackend, and PublicInbox::HTTP and finding
> > nothing but cleanups and documentation improvements; it seems
> > the leaks affecting lore is down to bugs in Perl 5.16.3.
> >
> > After removing the warning for Deflater being missing in
> > d883d4a93b23be134038e28f421eafca70c3d838
> > ("httpd: get rid of Deflater warning"), I missed that my
> > own CentOS 7 VM was missing that module so was unable to
> > reproduce the FD leaks :x
> >
> > The first patch is a straightforward workaround that I was
> > able to test without Plack::Middleware::Deflater being installed.
> >
> > The second patch stops loading Deflater in httpd on Perls
> > earlier than 5.18. (But I haven't built or tested 5.18 myself).
> > Enabling gzip in varnish will be needed for 5.16 users.
>
> Yay, I can confirm that the latest master fixes all the FD leaks that have
> been plaguing lore.kernel.org for the past few weeks. The number of pipes is
> stable and there are no (deleted) tempfiles showing up. Thanks so much for
> this!
Good to know! Also, how's memory use?
It ought to be stable if Perl is doing the right thing; but I
haven't tested all the endpoints thoroughly on 5.16.3
(especially the mbox.gz ones)
> Can you elaborate on gzip+varnish changes? I'm assuming it's something as
> simple as:
>
> @@ -63,6 +63,10 @@
> } else {
> /* short TTL for up-to-dateness, our PSGI is not that slow */
> set beresp.ttl = 10s;
> + /* Compress text responses on CentOS7 */
> + if (beresp.http.content-type ~ "text") {
> + set beresp.do_gzip = true;
> + }
*shrug* Whatever the varnish docs says :)
I still prefer to do gzip in the Perl process to minimize IPC
overhead (and to eventually make things easier-to-deploy w/o
needing Varnish).
I think I can add gzip support to WwwStream and WwwAtomStream
pretty easily, and we're already using a custom gzipper for
the mboxrd endpoints.
^ permalink raw reply [relevance 0%]
* Re: [PATCH 0/2] leak workarounds for Perl 5.16 on CentOS/RHEL 7
2019-09-26 1:50 6% [PATCH 0/2] leak workarounds for Perl 5.16 on CentOS/RHEL 7 Eric Wong
@ 2019-09-26 12:36 0% ` Konstantin Ryabitsev
2019-09-26 21:16 0% ` Eric Wong
0 siblings, 1 reply; 5+ results
From: Konstantin Ryabitsev @ 2019-09-26 12:36 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
On Thu, Sep 26, 2019 at 01:50:36AM +0000, Eric Wong wrote:
>After many hours of reviewing our code in PublicInbox::Qspawn,
>PublicInbox::GitHTTPBackend, and PublicInbox::HTTP and finding
>nothing but cleanups and documentation improvements; it seems
>the leaks affecting lore is down to bugs in Perl 5.16.3.
>
>After removing the warning for Deflater being missing in
>d883d4a93b23be134038e28f421eafca70c3d838
>("httpd: get rid of Deflater warning"), I missed that my
>own CentOS 7 VM was missing that module so was unable to
>reproduce the FD leaks :x
>
>The first patch is a straightforward workaround that I was
>able to test without Plack::Middleware::Deflater being installed.
>
>The second patch stops loading Deflater in httpd on Perls
>earlier than 5.18. (But I haven't built or tested 5.18 myself).
>Enabling gzip in varnish will be needed for 5.16 users.
Yay, I can confirm that the latest master fixes all the FD leaks that
have been plaguing lore.kernel.org for the past few weeks. The number of
pipes is stable and there are no (deleted) tempfiles showing up. Thanks
so much for this!
Can you elaborate on gzip+varnish changes? I'm assuming it's something
as simple as:
@@ -63,6 +63,10 @@
} else {
/* short TTL for up-to-dateness, our PSGI is not that slow */
set beresp.ttl = 10s;
+ /* Compress text responses on CentOS7 */
+ if (beresp.http.content-type ~ "text") {
+ set beresp.do_gzip = true;
+ }
}
return (deliver);
}
-K
^ permalink raw reply [relevance 0%]
* [PATCH 0/2] leak workarounds for Perl 5.16 on CentOS/RHEL 7
@ 2019-09-26 1:50 6% Eric Wong
2019-09-26 12:36 0% ` Konstantin Ryabitsev
0 siblings, 1 reply; 5+ results
From: Eric Wong @ 2019-09-26 1:50 UTC (permalink / raw)
To: meta
After many hours of reviewing our code in PublicInbox::Qspawn,
PublicInbox::GitHTTPBackend, and PublicInbox::HTTP and finding
nothing but cleanups and documentation improvements; it seems
the leaks affecting lore is down to bugs in Perl 5.16.3.
After removing the warning for Deflater being missing in
d883d4a93b23be134038e28f421eafca70c3d838
("httpd: get rid of Deflater warning"), I missed that my
own CentOS 7 VM was missing that module so was unable to
reproduce the FD leaks :x
The first patch is a straightforward workaround that I was
able to test without Plack::Middleware::Deflater being installed.
The second patch stops loading Deflater in httpd on Perls
earlier than 5.18. (But I haven't built or tested 5.18 myself).
Enabling gzip in varnish will be needed for 5.16 users.
Independently of this fix, I've long been considering replacing
Deflater with a buffer-to-gzip layer which would reduce memory
pressure from intermediate uncompressed strings. This might
serve as impetus to move that idea along (and of course I'd
test it heavily under CentOS 7 :>).
Anybody using custom .psgi files on Perl 5.16.x will need to
make adjustments to disable the Deflater in that.
Eric Wong (2):
ds: workaround a memory leak in Perl 5.16.x
httpd: disable Deflater middleware by default on Perl <5.18
lib/PublicInbox/DS.pm | 9 ++++++---
script/public-inbox-httpd | 8 +++++++-
2 files changed, 13 insertions(+), 4 deletions(-)
--
EW
^ permalink raw reply [relevance 6%]
* default PSGI middlewares (was: httpd: get rid of Deflater warning)
2019-05-05 20:42 7% [PATCH] httpd: get rid of Deflater warning Eric Wong
@ 2019-05-05 21:28 7% ` Eric Wong
0 siblings, 0 replies; 5+ results
From: Eric Wong @ 2019-05-05 21:28 UTC (permalink / raw)
To: meta
Now, the ReverseProxy middleware may be invalid (or become less
valid) in the future as well: TLS may come to NNTP and we might
as well support HTTPS natively while we're at it.
I can't drop middlewares by default since it would break
existing installations which don't rely on a .psgi config;
but there can be options to disable it... (not that I'm a
fan of adding more options).
Keeping ReverseProxy in the default middleware stack along with
the existing warning would probably be the right thing to do.
Worst case somebody malicious could do is change logging of
IP addresses. Maybe the warning could be silenced if listening
on 0.0.0.0:{443,80}
^ permalink raw reply [relevance 7%]
* [PATCH] httpd: get rid of Deflater warning
@ 2019-05-05 20:42 7% Eric Wong
2019-05-05 21:28 7% ` default PSGI middlewares (was: httpd: get rid of Deflater warning) Eric Wong
0 siblings, 1 reply; 5+ results
From: Eric Wong @ 2019-05-05 20:42 UTC (permalink / raw)
To: meta
Deflating responses may be done by the reverse proxy (e.g. varnish
or nginx), so the warning for it could be invalid.
---
examples/public-inbox.psgi | 2 --
script/public-inbox-httpd | 2 --
2 files changed, 4 deletions(-)
diff --git a/examples/public-inbox.psgi b/examples/public-inbox.psgi
index 8886d7f..0b8500d 100644
--- a/examples/public-inbox.psgi
+++ b/examples/public-inbox.psgi
@@ -25,8 +25,6 @@ builder {
application/atom+xml
)]
};
- $@ and warn
-"Plack::Middleware::Deflater missing, bandwidth will be wasted\n";
# Enable to ensure redirects and Atom feed URLs are generated
# properly when running behind a reverse proxy server which
diff --git a/script/public-inbox-httpd b/script/public-inbox-httpd
index 47e38ec..56551ed 100755
--- a/script/public-inbox-httpd
+++ b/script/public-inbox-httpd
@@ -32,8 +32,6 @@ my $refresh = sub {
application/atom+xml
)]
};
- $@ and warn
-"Plack::Middleware::Deflater missing, bandwidth will be wasted\n";
eval { enable 'ReverseProxy' };
$@ and warn
--
EW
^ permalink raw reply related [relevance 7%]
Results 1-5 of 5 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2019-05-05 20:42 7% [PATCH] httpd: get rid of Deflater warning Eric Wong
2019-05-05 21:28 7% ` default PSGI middlewares (was: httpd: get rid of Deflater warning) Eric Wong
2019-09-26 1:50 6% [PATCH 0/2] leak workarounds for Perl 5.16 on CentOS/RHEL 7 Eric Wong
2019-09-26 12:36 0% ` Konstantin Ryabitsev
2019-09-26 21:16 0% ` 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).