[PATCH 0/4] compression-related stuff

[PATCH 0/4] compression-related stuff

[Eric Wong]

1/4 avoids some log noise due to expected occurences.  The
others are small cleanups and doc improvements noticed while
doing other WIP stuff...

3/4 ought to make things easier for my puny brain :x

Eric Wong (4):
  www: gzip_filter: gracefully handle socket ->write failures
  www: gzip_filter: update a few comments
  ds: use ->dflush to distinguish from ->zflush
  www: simplify GzipFilter->zflush callers

 lib/PublicInbox/DS.pm         |  4 ++--
 lib/PublicInbox/DSdeflate.pm  |  4 ++--
 lib/PublicInbox/GzipFilter.pm | 10 ++++++----
 lib/PublicInbox/IMAP.pm       |  2 +-
 lib/PublicInbox/Mbox.pm       |  3 +--
 lib/PublicInbox/NNTP.pm       |  2 +-
 lib/PublicInbox/WwwText.pm    |  6 +-----
 7 files changed, 14 insertions(+), 17 deletions(-)

[PATCH 1/4] www: gzip_filter: gracefully handle socket ->write failures

[Eric Wong]

Socket ->write failures are expected and common for TCP traffic,
especially if it's facing unreliable remote connections.  So
just bail out silently if our {gz} field was already clobbered
during the small bit of recursion we hit on ->write failures
from async responses.

This ought to fix some GzipFilter::zflush errors (via $forward
->close from PublicInbox::HTTP) I've been noticing on
deployments running -netd.  I'm still unsure as to why I hadn't
seen them before, but it might've only been ignorance on my
part...

Link: https://public-inbox.org/meta/20220802065436.GA13935@dcvr/
---
 lib/PublicInbox/GzipFilter.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm
index e37f1f76..c586d2f8 100644
--- a/lib/PublicInbox/GzipFilter.pm
+++ b/lib/PublicInbox/GzipFilter.pm
@@ -149,10 +149,11 @@ sub zflush ($;$) {
 	my $zbuf = delete $self->{zbuf};
 	my $gz = delete $self->{gz};
 	my $err;
-	if (defined $_[1]) {
+	if (defined $_[1]) { # it's a bug iff $gz is undef w/ $_[1]
 		$err = $gz->deflate($_[1], $zbuf);
 		die "gzip->deflate: $err" if $err != Z_OK;
 	}
+	$gz // return; # not a bug, recursing on DS->write failure
 	$err = $gz->flush($zbuf);
 	die "gzip->flush: $err" if $err != Z_OK;
 	$zbuf;

[PATCH 2/4] www: gzip_filter: update a few comments

[Eric Wong]

A few things I noticed while reviewing and evaluating
the PSGI code for JMAP support.
---
 lib/PublicInbox/GzipFilter.pm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm
index c586d2f8..d41748c4 100644
--- a/lib/PublicInbox/GzipFilter.pm
+++ b/lib/PublicInbox/GzipFilter.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 #
 # In public-inbox <=1.5.0, public-inbox-httpd favored "getline"
@@ -116,6 +116,7 @@ sub translate ($$) {
 	}
 }
 
+# returns PublicInbox::HTTP::{Chunked,Identity}
 sub http_out ($) {
 	my ($self) = @_;
 	$self->{http_out} // do {
@@ -183,7 +184,7 @@ sub bail  {
 # this is public-inbox-httpd-specific
 sub async_blob_cb { # git->cat_async callback
 	my ($bref, $oid, $type, $size, $self) = @_;
-	my $http = $self->{env}->{'psgix.io'};
+	my $http = $self->{env}->{'psgix.io'}; # PublicInbox::HTTP
 	$http->{forward} or return; # client aborted
 	my $smsg = $self->{smsg} or bail($self, 'BUG: no smsg');
 	if (!defined($oid)) {
@@ -195,7 +196,7 @@ sub async_blob_cb { # git->cat_async callback
 	$smsg->{blob} eq $oid or bail($self, "BUG: $smsg->{blob} != $oid");
 	eval { $self->async_eml(PublicInbox::Eml->new($bref)) };
 	bail($self, "E: async_eml: $@") if $@;
-	if ($self->{-low_prio}) {
+	if ($self->{-low_prio}) { # run via PublicInbox::WWW::event_step
 		push(@{$self->{www}->{-low_prio_q}}, $self) == 1 and
 				PublicInbox::DS::requeue($self->{www});
 	} else {

[PATCH 3/4] ds: use ->dflush to distinguish from ->zflush

[Eric Wong]

->zflush is already for GzipFilter in PublicInbox::WWW,
while we use DEFLATE for NNTP and IMAP.  This ought to
make the code easier-to-follow.
---
 lib/PublicInbox/DS.pm        | 4 ++--
 lib/PublicInbox/DSdeflate.pm | 4 ++--
 lib/PublicInbox/IMAP.pm      | 2 +-
 lib/PublicInbox/NNTP.pm      | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index ef483aac..77e2e5e9 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -648,7 +648,7 @@ sub shutdn ($) {
     }
 }
 
-sub zflush {} # overridden by DSdeflate
+sub dflush {} # overridden by DSdeflate
 sub compressed {} # overridden by DSdeflate
 sub long_response_done {} # overridden by Net::NNTP
 
@@ -682,7 +682,7 @@ sub requeue_once {
 	my ($self) = @_;
 	# COMPRESS users all share the same DEFLATE context.
 	# Flush it here to ensure clients don't see each other's data
-	$self->zflush;
+	$self->dflush;
 
 	# no recursion, schedule another call ASAP,
 	# but only after all pending writes are done.
diff --git a/lib/PublicInbox/DSdeflate.pm b/lib/PublicInbox/DSdeflate.pm
index bf0ecd38..639690e2 100644
--- a/lib/PublicInbox/DSdeflate.pm
+++ b/lib/PublicInbox/DSdeflate.pm
@@ -30,7 +30,7 @@ my %IN_OPT = (
 my ($zout, $zbuf);
 {
 	my $err;
-	$zbuf = \(my $initial = ''); # replaced by $next in zflush/write
+	$zbuf = \(my $initial = ''); # replaced by $next in dflush/write
 	($zout, $err) = Compress::Raw::Zlib::Deflate->new(
 		# nnrpd (INN) and Compress::Raw::Zlib favor MemLevel=9,
 		# the zlib C library and git use MemLevel=8 as the default
@@ -100,7 +100,7 @@ sub msg_more ($$) {
 	1;
 }
 
-sub zflush ($) {
+sub dflush ($) {
 	my ($self) = @_;
 
 	my $deflated = $zbuf;
diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 805f1102..0f0f9b3a 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -595,7 +595,7 @@ sub fetch_blob_cb { # called by git->cat_async via ibx_async_cat
 					\&fetch_blob_cb, $fetch_arg);
 	}
 	fetch_run_ops($self, $smsg, $bref, $ops, $partial);
-	$pre ? $self->zflush : $self->requeue_once;
+	$pre ? $self->dflush : $self->requeue_once;
 }
 
 sub emit_rfc822 {
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index b223eb07..791fe2a9 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -556,7 +556,7 @@ sub blob_cb { # called by git->cat_async via ibx_async_cat
 		$self->close;
 		die "BUG: bad code: $r";
 	}
-	$self->write(\".\r\n"); # flushes (includes ->zflush)
+	$self->write(\".\r\n"); # flushes (includes ->dflush)
 	$self->requeue;
 }
 

[PATCH 4/4] www: simplify GzipFilter->zflush callers

[Eric Wong]

->zflush can take a buffer arg, so there's no need to
make a separate call to ->translate in some cases.
---
 lib/PublicInbox/Mbox.pm    | 3 +--
 lib/PublicInbox/WwwText.pm | 6 +-----
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm
index b977308d..e65f38f0 100644
--- a/lib/PublicInbox/Mbox.pm
+++ b/lib/PublicInbox/Mbox.pm
@@ -24,8 +24,7 @@ sub getline {
 	if ($n) {
 		$ctx->translate(msg_body($eml));
 	} else { # last message
-		$ctx->zmore(msg_body($eml));
-		$ctx->zflush;
+		$ctx->zflush(msg_body($eml));
 	}
 }
 
diff --git a/lib/PublicInbox/WwwText.pm b/lib/PublicInbox/WwwText.pm
index 2937c333..369328ee 100644
--- a/lib/PublicInbox/WwwText.pm
+++ b/lib/PublicInbox/WwwText.pm
@@ -38,11 +38,7 @@ sub get_text {
 	}
 	my $env = $ctx->{env};
 	if ($raw) {
-		if ($code == 200) {
-			my $gzf = gzf_maybe($hdr, $env);
-			$txt = $gzf->translate($txt);
-			$txt .= $gzf->zflush;
-		}
+		$txt = gzf_maybe($hdr, $env)->zflush($txt) if $code == 200;
 		$hdr->[3] = length($txt);
 		return [ $code, $hdr, [ $txt ] ]
 	}