[PATCH] PR26810: debuginfod partial error handling

[Frank Ch. Eigler]

Hi -


commit 529f5bbf5687c1ad66040bbe41bd9dd050fa1890 (HEAD -> master)
Author: Frank Ch. Eigler <fche@redhat.com>
Date:   Thu Oct 29 12:20:51 2020 -0400

    PR26810: debuginfod should tolerate some absence/renaming sans grooming
    
    debuginfod now knows to handle a case where a buildid search is
    satisfiable from more than one source (e.g., archive location), but
    some of them are invalid.  New exception catching beneath the sqlite
    scanning loop ensures all possible matches are scanned in case of
    errors.
    
    Signed-off-by: Frank Ch. Eigler <fche@redhat.com>

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 558572333fb8..9eb9c813dd23 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,11 @@
+2020-10-29  Frank Ch. Eigler  <fche@redhat.com>
+
+	PR26810
+	* debuginfod.cxx (handle_buildid_*_match): Throw exceptions for
+	more lower level libc errors.
+	(handle_buildid_match): Catch & report exceptions but return 0
+	for continued iteration in the caller.
+
 2020-10-25  Mark Wielaard  <mark@klomp.org>
 
 	* debuginfod-client.c (debuginfod_query_server): Translate
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 2b68ff1f052d..9da65d87d601 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -906,12 +906,7 @@ handle_buildid_f_match (bool internal_req_t,
   (void) internal_req_t; // ignored
   int fd = open(b_source0.c_str(), O_RDONLY);
   if (fd < 0)
-    {
-      if (verbose)
-        obatched(clog) << "cannot open " << b_source0 << endl;
-      // if still missing, a periodic groom pass will delete this buildid record
-      return 0;
-    }
+    throw libc_exception (errno, string("open ") + b_source0);
 
   // NB: use manual close(2) in error case instead of defer_dtor, because
   // in the normal case, we want to hand the fd over to libmicrohttpd for
@@ -921,10 +916,8 @@ handle_buildid_f_match (bool internal_req_t,
   int rc = fstat(fd, &s);
   if (rc < 0)
     {
-      if (verbose)
-        clog << "cannot fstat " << b_source0 << endl;
       close(fd);
-      return 0;
+      throw libc_exception (errno, string("fstat ") + b_source0);
     }
 
   if ((int64_t) s.st_mtime != b_mtime)
@@ -1470,12 +1463,21 @@ handle_buildid_match (bool internal_req_p,
                       const string& b_source1,
                       int *result_fd)
 {
-  if (b_stype == "F")
-    return handle_buildid_f_match(internal_req_p, b_mtime, b_source0, result_fd);
-  else if (b_stype == "R")
-    return handle_buildid_r_match(internal_req_p, b_mtime, b_source0, b_source1, result_fd);
-  else
-    return 0;
+  try
+    {
+      if (b_stype == "F")
+        return handle_buildid_f_match(internal_req_p, b_mtime, b_source0, result_fd);
+      else if (b_stype == "R")
+        return handle_buildid_r_match(internal_req_p, b_mtime, b_source0, b_source1, result_fd);
+    }
+  catch (const reportable_exception &e)
+    {
+      e.report(clog);
+      // Report but swallow libc etc. errors here; let the caller
+      // iterate to other matches of the content.
+    }
+  
+  return 0;
 }
 
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index e9d1e2606d68..f57db304bfeb 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-29  Frank Ch. Eigler  <fche@redhat.com>
+
+	PR26810
+	* run-debuginfod-find.sh: Add tests for successful archive fetches across
+	renamed RPMs, even without grooming.
+
 2020-10-19  Mark Wielaard  <mark@klomp.org>
 
 	* addrcfi.c (print_register): Make ops_mem 3 elements.
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 79976f70dc92..52def36438bb 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -357,6 +357,38 @@ testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID2
 
 ########################################################################
 
+# PR26810: Now rename some files in the R directory, then rescan, so
+# there are two copies of the same buildid in the index, one for the
+# no-longer-existing file name, and one under the new name.
+
+# run a groom cycle to force server to drop its fdcache
+kill -USR2 $PID1  # groom cycle
+wait_ready $PORT1 'thread_work_total{role="groom"}' 3
+# move it around a couple of times to make it likely to hit a nonexistent entry during iteration
+mv R/debuginfod-rpms/rhel7 R/debuginfod-rpms/rhel7renamed
+kill -USR1 $PID1  # scan cycle
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 6
+wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
+wait_ready $PORT1 'thread_busy{role="scan"}' 0
+mv R/debuginfod-rpms/rhel7renamed R/debuginfod-rpms/rhel7renamed2
+kill -USR1 $PID1  # scan cycle
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 7
+wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
+wait_ready $PORT1 'thread_busy{role="scan"}' 0
+mv R/debuginfod-rpms/rhel7renamed2 R/debuginfod-rpms/rhel7renamed3
+kill -USR1 $PID1  # scan cycle
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 8
+wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
+wait_ready $PORT1 'thread_busy{role="scan"}' 0
+
+# retest rhel7
+archive_test bc1febfd03ca05e030f0d205f7659db29f8a4b30 /usr/src/debug/hello-1.0/hello.c $SHA
+archive_test f0aa15b8aba4f3c28cac3c2a73801fefa644a9f2 /usr/src/debug/hello-1.0/hello.c $SHA
+
+egrep '(libc.error.*rhel7)|(bc1febfd03ca)|(f0aa15b8aba)' vlog4
+
+########################################################################
+
 # Federation mode
 
 # find another unused port