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 558572333fb807c2cc8302ebad3bddce15c3a1b0..9eb9c813dd230ed4edd5415788e27b02028c6a5a 100644 (file)
--- 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 2b68ff1f052d4cace30a1f51a9e3bda4ce312198..9da65d87d601771b2dff23b3f73e8c7bcf770dc1 100644 (file)
--- 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 d0149bab59e1ba54444a8213a23af76bc0bdc65e..6ea75d25f076c582d2c4a245f6a3ad9cf4969ca8 100644 (file)
--- 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-25  Mark Wielaard  <mark@klomp.org>
 
        * read_unaligned.c: New test.

diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 79976f70dc920516b45ccbe0f3748075a9ab0a84..52def36438bbac014fc6ec63f159fe74b116bed0 100755 (executable)
--- 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