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.

3 files changed, 21 insertions, 12 deletions

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;
 }