From dc03cabb5d167618797e9e8a6ec615bda7b0638b Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Sat, 19 Sep 2020 09:37:11 +0000
Subject: gcf2: transparently retry on missing OID

Since we only get OIDs from trusted local data sources
(over.sqlite3), we can safely retry within the -gcf2 process
without worry about clients spamming us with requests for
invalid OIDs and triggering reopens.
---
 lib/PublicInbox/Gcf2Client.pm  | 11 ++++++++---
 lib/PublicInbox/Git.pm         |  5 +++--
 lib/PublicInbox/gcf2_libgit2.h | 17 ++++++++++-------
 3 files changed, 21 insertions(+), 12 deletions(-)

(limited to 'lib')

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 71fbb1d1..5120698f 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -7,11 +7,13 @@ use PublicInbox::Spawn qw(popen_rd);
 use IO::Handle ();
 
 sub new {
-	my $self = shift->SUPER::new('/nonexistent');
+	my ($rdr) = @_;
+	my $self = bless {}, __PACKAGE__;
 	my ($out_r, $out_w);
 	pipe($out_r, $out_w) or $self->fail("pipe failed: $!");
-	my $cmd = [ 'public-inbox-gcf2' ];
-	@$self{qw(in pid)} = popen_rd($cmd, undef, { 0 => $out_r });
+	$rdr //= {};
+	$rdr->{0} = $out_r;
+	@$self{qw(in pid)} = popen_rd(['public-inbox-gcf2'], undef, $rdr);
 	$self->{inflight} = [];
 	$self->{out} = $out_w;
 	fcntl($out_w, 1031, 4096) if $^O eq 'linux'; # 1031: F_SETPIPE_SZ
@@ -32,4 +34,7 @@ sub add_git_dir {
 				$self->fail("write error: $!");
 }
 
+# always false, since -gcf2 retries internally
+sub alternates_changed {}
+
 1;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index a7ba57f9..b49b5bd3 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -192,7 +192,8 @@ sub cat_async_step ($$) {
 		chop($$bref) eq "\n" or fail($self, 'LF missing after blob');
 	} elsif ($head =~ / missing$/) {
 		# ref($req) indicates it's already been retried
-		if (!ref($req) && !$in_cleanup && alternates_changed($self)) {
+		# -gcf2 retries internally, so it never hits this path:
+		if (!ref($req) && !$in_cleanup && $self->alternates_changed) {
 			return cat_async_retry($self, $inflight,
 						$req, $cb, $arg);
 		}
@@ -394,7 +395,7 @@ sub pub_urls {
 
 sub cat_async_begin {
 	my ($self) = @_;
-	cleanup($self) if alternates_changed($self);
+	cleanup($self) if $self->alternates_changed;
 	batch_prepare($self);
 	die 'BUG: already in async' if $self->{inflight};
 	$self->{inflight} = [];
diff --git a/lib/PublicInbox/gcf2_libgit2.h b/lib/PublicInbox/gcf2_libgit2.h
index d9c79cf9..800c6bad 100644
--- a/lib/PublicInbox/gcf2_libgit2.h
+++ b/lib/PublicInbox/gcf2_libgit2.h
@@ -52,9 +52,13 @@ void add_alternate(SV *self, const char *objects_path)
 	croak_if_err(rc, "git_odb_add_disk_alternate");
 }
 
-/* this requires an unabbreviated git OID */
 #define CAPA(v) (sizeof(v) / sizeof((v)[0]))
-void cat_oid(SV *self, int fd, SV *oidsv)
+
+/*
+ * returns true on success, false on failure
+ * this requires an unabbreviated git OID
+ */
+int cat_oid(SV *self, int fd, SV *oidsv)
 {
 	/*
 	 * adjust when libgit2 gets SHA-256 support, we return the
@@ -89,11 +93,8 @@ void cat_oid(SV *self, int fd, SV *oidsv)
 					git_object_type2string(
 						git_odb_object_type(object)),
 					vec[1].iov_len);
-	} else {
-		vec[0].iov_base = oidptr;
-		vec[0].iov_len = oidlen;
-		vec[1].iov_base = " missing";
-		vec[1].iov_len = strlen(vec[1].iov_base);
+	} else { /* caller retries */
+		nvec = 0;
 	}
 	while (nvec && !err) {
 		ssize_t w = writev(fd, vec + CAPA(vec) - nvec, nvec);
@@ -136,4 +137,6 @@ void cat_oid(SV *self, int fd, SV *oidsv)
 		git_odb_object_free(object);
 	if (err)
 		croak("writev error: %s", strerror(err));
+
+	return rc == GIT_OK;
 }
-- 
cgit v1.2.3-24-ge0c7