Re: NNTP COMPRESS clients? RFC 8054

[Eric Wong <e@80x24.org>]

Julien ÉLIE <julien@trigofacile.com> wrote:
> Hi Eric,
> 
> Eric Wong wrote:
> > I've prepared a patch to the
> > Perl Net::NNTP module and am hoping I got everything right...
> > I've only lightly tested it against news.gmane.org which runs inn:
> > 
> >    https://rt.cpan.org/Ticket/Display.html?id=129967
> 
> I've just taken the time to have a look at it.  A few comments below:

Hi Julien,

Fwiw, I was more interested in making sure Net::NNTP client was
right OK in that RT ticket:

	git clone https://80x24.org/perl-libnet.git
	(nntp-compress branch)

But it looks like you've looked at the public-inbox-nntpd server
code at `git clone https://public-inbox.org/', but I appreciate
the comments either way :)

+Cc: meta@public-inbox.org since it's on topic, there..

> +	LINE_MAX => 512, # RFC 977 section 2.3
> 
> Now "RFC 3977 Section 3.1" :)

Right, though I think a lot of the Net::NNTP client code (and
maybe a lot of other clients) is still RFC 977, so I'd like to
retain compatibility (and I'm still catching up and missing
some commands)

> +	$err == Z_OK or die "Failed to initialize zlib deflate stream: $err";
> 
> Shouldn't a 403 response code be offered?  The connection then goes on,
> uncompressed.

Right, the inflate stream now returns 403:

  https://public-inbox.org/meta/20190707070831.fafviqilsywuc5bw@dcvr/

I also made a another change to do a single deflate stream at
startup to save a bunch of memory at the expense of reduced
compression.  If that fails, the rest of the NNTP server
continues w/o that capability:

  https://public-inbox.org/meta/20190705225339.5698-5-e@80x24.org/

> +# RSS usage for 10K idle-but-did-something NNTP clients on 64-bit:
> +#   TLS + DEFLATE :  1.8 GB  (MemLevel=9, 1.2 GB with MemLevel=8)
> +#   TLS only      :  <200MB
> +#   plain         :   <50MB
> 
> How did you test those 10k connections?  (with your test suite
> nntpd-validate.t?)

It's not part of the test suite, yet.   I do notice a drastic
memory difference on the server depending on whether the client
is using Danga::Socket or PublicInbox::DS(*)

Will report back on that later (though maybe much later...)

(*) - the latter is a gutted, API-unstable fork of Danga::Socket
      which allows use of EPOLLONESHOT/EV_DISPATCH

> +	# nnrpd (INN) and Compress::Raw::Zlib favor MemLevel=9,
> +	# but the zlib C library and git use MemLevel=8
> +	# as the default.  Using 8 drops our memory use with 10K
> +	# TLS clients from 1.8 GB to 1.2 GB, but...
> +	# FIXME: sometimes clients fail with 8, so we use 9
> +	# -MemLevel => 9,
> 
> Strange that clients fail with mem level 8.  Seems like a bug with such
> clients.
> Hopefully nnrpd uses mem level 9 anyway.

Not a bug in clients; it was a stupid bug on the server not
realizing Deflate->new would return an array when used inside []:

  https://public-inbox.org/meta/20190707070831.fafviqilsywuc5bw@dcvr/

> +	# Net::NNTP emit different line-endings depending on TLS or not...:
> +	$data =~ tr/\r//d;
> 
> Shouldn't it be fixed in Net::NNTP?

Maybe, but it's not a big issue AFAIK.

I'm still traumatized from dealing with method resolution
confusion while implementing COMPRESS on the client side :x
I documented some of this the other day at:

  https://rt.cpan.org/Public/Bug/Display.html?id=129966#txn-1854816

It'll be up to the libnet maintainer(s) on whether to fix it.

> > I also implemented it server-side in public-inbox-nntpd[1],
> > which powers news.public-inbox.org, but there's a danger I'm
> > implementing something wrong on both ends in that case :x
> 
> Time to implement it in nntp.perl.org then :)
> It runs Colobus, an NNTP news server written in Perl by Ask Bjørn Hansen.

My plan is to keep public-inbox-nntpd read-only; since (AFAIK)
NNTP lacks good spam filtering and most news->mail gateways get
shut down because of that.  But it might be easy for Ask or
anybody else to implement posting if they desire...

I also don't want to hassle him or anybody at perl.org with
migrating, given given how little load it sees:

  http://nntp.perl.org/group/perl.perl5.porters/255487