Re: [PATCH v4 0/7] debuginfod: speed up extraction from kernel debuginfo packages by 200x

2024-07-23 Thread Aaron Merey
Hi Omar,

On Fri, Jul 19, 2024 at 2:24 PM Omar Sandoval  wrote:
>
> From: Omar Sandoval 
>
> This is v4 of my patch series optimizing debuginfod for kernel
> debuginfo.  v1 is here [1], v2 is here [2], v3 is here [3].  The only
> changes from v3 in this version are fixing a bogus maybe-uninitialized
> error on the Debian build and adding the new test files to EXTRA_DIST so
> that make distcheck passes.
>
> Thanks,
> Omar
>
> 1: https://sourceware.org/pipermail/elfutils-devel/2024q3/007191.html
> 2: https://sourceware.org/pipermail/elfutils-devel/2024q3/007208.html
> 3: https://sourceware.org/pipermail/elfutils-devel/2024q3/007243.html

Thanks for working on these patches.  I ran v4 on the sourceware buildbots.
The new testcase fails on debian-ppc64 [1], debian-i386 [2] and
fedora-s390x [3].  There was a 4th failing buildbot but AFAICT the failure
is unrelated to your patches.

Do you mind taking a look at this?  The patches otherwise LGTM and I'll
merge them once this is passing on the buildbots.

Aaron

[1] https://builder.sourceware.org/buildbot/#/builders/214/builds/190
[2] https://builder.sourceware.org/buildbot/#/builders/210/builds/194
[3] https://builder.sourceware.org/buildbot/#/builders/211/builds/185



Re: [PATCH v4 0/7] debuginfod: speed up extraction from kernel debuginfo packages by 200x

2024-07-23 Thread Omar Sandoval
On Tue, Jul 23, 2024 at 05:47:50PM -0400, Aaron Merey wrote:
> Hi Omar,
> 
> On Fri, Jul 19, 2024 at 2:24 PM Omar Sandoval  wrote:
> >
> > From: Omar Sandoval 
> >
> > This is v4 of my patch series optimizing debuginfod for kernel
> > debuginfo.  v1 is here [1], v2 is here [2], v3 is here [3].  The only
> > changes from v3 in this version are fixing a bogus maybe-uninitialized
> > error on the Debian build and adding the new test files to EXTRA_DIST so
> > that make distcheck passes.
> >
> > Thanks,
> > Omar
> >
> > 1: https://sourceware.org/pipermail/elfutils-devel/2024q3/007191.html
> > 2: https://sourceware.org/pipermail/elfutils-devel/2024q3/007208.html
> > 3: https://sourceware.org/pipermail/elfutils-devel/2024q3/007243.html
> 
> Thanks for working on these patches.  I ran v4 on the sourceware buildbots.
> The new testcase fails on debian-ppc64 [1], debian-i386 [2] and
> fedora-s390x [3].  There was a 4th failing buildbot but AFAICT the failure
> is unrelated to your patches.
> 
> Do you mind taking a look at this?  The patches otherwise LGTM and I'll
> merge them once this is passing on the buildbots.

Thanks for the test run, and sorry for the churn.  It looks like these
are all failing with "error: no fdcache hits" because every addition to
the fdcache results in "fdcache emergency flush for filling tmpdir".  I
see that run-debuginfod-fd-prefetch-caches.sh has a workaround for this
which I'll copy.

Thanks,
Omar


[PATCH v5 1/7] debuginfod: fix skipping source file

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

dwarf_extract_source_paths explicitly skips source files that equal
"", but dwarf_filesrc may return a path like "dir/".
Check for and skip that case, too.

In particular, the test debuginfod RPMs have paths like this.  However,
the test cases didn't catch this because they have a bug, too: they
follow symlinks, which results in double-counting every file.  Fix that,
too.

Signed-off-by: Omar Sandoval 
---
 debuginfod/debuginfod.cxx | 3 ++-
 tests/run-debuginfod-archive-groom.sh | 2 +-
 tests/run-debuginfod-extraction.sh| 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 305edde8..92022f3d 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -3446,7 +3446,8 @@ dwarf_extract_source_paths (Elf *elf, set& 
debug_sourcefiles)
   if (hat == NULL)
 continue;
 
-  if (string(hat) == "") // gcc intrinsics, don't bother 
record
+  if (string(hat) == ""
+  || string_endswith(hat, "")) // gcc intrinsics, don't 
bother record
 continue;
 
   string waldo;
diff --git a/tests/run-debuginfod-archive-groom.sh 
b/tests/run-debuginfod-archive-groom.sh
index e2c394ef..0131158f 100755
--- a/tests/run-debuginfod-archive-groom.sh
+++ b/tests/run-debuginfod-archive-groom.sh
@@ -109,7 +109,7 @@ for i in $newrpms; do
 rpm2cpio ../$i | cpio -ivd;
 cd ..;
 done
-sourcefiles=$(find -name \*\\.debug \
+sourcefiles=$(find -name \*\\.debug -type f \
   | env LD_LIBRARY_PATH=$ldpath xargs \
 ${abs_top_builddir}/src/readelf --debug-dump=decodedline \
   | grep mtime: | wc --lines)
diff --git a/tests/run-debuginfod-extraction.sh 
b/tests/run-debuginfod-extraction.sh
index da6b25cf..f49dc6f6 100755
--- a/tests/run-debuginfod-extraction.sh
+++ b/tests/run-debuginfod-extraction.sh
@@ -94,7 +94,7 @@ for i in $newrpms; do
 rpm2cpio ../$i | cpio -ivd;
 cd ..;
 done
-sourcefiles=$(find -name \*\\.debug \
+sourcefiles=$(find -name \*\\.debug -type f \
   | env LD_LIBRARY_PATH=$ldpath xargs \
 ${abs_top_builddir}/src/readelf --debug-dump=decodedline \
   | grep mtime: | wc --lines)
-- 
2.45.2



[PATCH v5 0/7] debuginfod: speed up extraction from kernel debuginfo packages by 200x

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

This is v4 of my patch series optimizing debuginfod for kernel
debuginfo.  v1 is here [1], v2 is here [2], v3 is here [3], v4 is here
[4].  The only change from v4 in this version is adding --fdcache-mbs
and --fdcache-mintmp to the new test to fix some sporadic test failures.
Hopefully this version finally gets a clean test run.

Thanks,
Omar

1: https://sourceware.org/pipermail/elfutils-devel/2024q3/007191.html
2: https://sourceware.org/pipermail/elfutils-devel/2024q3/007208.html
3: https://sourceware.org/pipermail/elfutils-devel/2024q3/007243.html
4: https://sourceware.org/pipermail/elfutils-devel/2024q3/007255.html

Omar Sandoval (7):
  debuginfod: fix skipping  source file
  tests/run-debuginfod-fd-prefetch-caches.sh: disable fdcache limit
check
  debuginfod: factor out common code for responding from an archive
  debugifod: add new table and views for seekable archives
  debuginfod: optimize extraction from seekable xz archives
  debuginfod: populate _r_seekable on scan
  debuginfod: populate _r_seekable on request

 configure.ac  |   5 +
 debuginfod/Makefile.am|   2 +-
 debuginfod/debuginfod.cxx | 923 +++---
 tests/Makefile.am |  13 +-
 ...pressme-seekable-xz-dbgsym_1.0-1_amd64.deb | Bin 0 -> 6288 bytes
 ...compressme-seekable-xz_1.0-1.debian.tar.xz | Bin 0 -> 1440 bytes
 .../compressme-seekable-xz_1.0-1.dsc  |  19 +
 .../compressme-seekable-xz_1.0-1_amd64.deb| Bin 0 -> 6208 bytes
 .../compressme-seekable-xz_1.0.orig.tar.xz| Bin 0 -> 7160 bytes
 .../compressme-seekable-xz-1.0-1.src.rpm  | Bin 0 -> 15880 bytes
 .../compressme-seekable-xz-1.0-1.x86_64.rpm   | Bin 0 -> 31873 bytes
 ...sme-seekable-xz-debuginfo-1.0-1.x86_64.rpm | Bin 0 -> 21917 bytes
 ...e-seekable-xz-debugsource-1.0-1.x86_64.rpm | Bin 0 -> 7961 bytes
 tests/run-debuginfod-archive-groom.sh |   2 +-
 tests/run-debuginfod-extraction.sh|   2 +-
 tests/run-debuginfod-fd-prefetch-caches.sh|   4 +
 tests/run-debuginfod-seekable.sh  | 192 
 17 files changed, 1017 insertions(+), 145 deletions(-)
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz-dbgsym_1.0-1_amd64.deb
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0-1.debian.tar.xz
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0-1.dsc
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0-1_amd64.deb
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0.orig.tar.xz
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-1.0-1.src.rpm
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-1.0-1.x86_64.rpm
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-debuginfo-1.0-1.x86_64.rpm
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-debugsource-1.0-1.x86_64.rpm
 create mode 100755 tests/run-debuginfod-seekable.sh

-- 
2.45.2



[PATCH v5 2/7] tests/run-debuginfod-fd-prefetch-caches.sh: disable fdcache limit check

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

Since commit acd9525e93d7 ("PR31265 - rework debuginfod archive-extract
fdcache"), the fdcache limit is only applied when a new file is interned
and it has been at least 10 seconds since the limit was last applied.
This means that the fdcache can go over the limit temporarily.

run-debuginfod-fd-prefetch-caches.sh happens to avoid tripping over this
because of lucky sizes of the files used in the test.  However, adding
new files for an upcoming test exposed this failure.

Disable this part of the test for now.

Signed-off-by: Omar Sandoval 
---
 tests/run-debuginfod-fd-prefetch-caches.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/run-debuginfod-fd-prefetch-caches.sh 
b/tests/run-debuginfod-fd-prefetch-caches.sh
index 3db78ade..90730555 100755
--- a/tests/run-debuginfod-fd-prefetch-caches.sh
+++ b/tests/run-debuginfod-fd-prefetch-caches.sh
@@ -99,6 +99,9 @@ kill $PID1
 wait $PID1
 PID1=0
 
+# Since we now only limit the fd cache every 10 seconds, it can temporarily go
+# over the limit.  That makes this part of the test unreliable.
+if false; then
 #
 # Test mb limit on fd cache
 #
@@ -148,3 +151,4 @@ kill $PID1
 wait $PID1
 PID1=0
 exit 0
+fi
-- 
2.45.2



[PATCH v5 7/7] debuginfod: populate _r_seekable on request

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

Since the schema change adding _r_seekable was done in a backward
compatible way, seekable archives that were previously scanned will not
be in _r_seekable.  Whenever an archive is going to be extracted to
satisfy a request, check if it is seekable.  If so, populate _r_seekable
while extracting it so that future requests use the optimized path.

The next time that BUILDIDS is bumped, all archives will be checked at
scan time.  At that point, checking again will be unnecessary and this
commit (including the test case modification) can be reverted.

Signed-off-by: Omar Sandoval 
---
 debuginfod/debuginfod.cxx| 76 +---
 tests/run-debuginfod-seekable.sh | 48 
 2 files changed, 118 insertions(+), 6 deletions(-)

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 5fe2db0c..fb7873ae 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -2740,6 +2740,7 @@ handle_buildid_r_match (bool internal_req_p,
 }
 
   // no match ... look for a seekable entry
+  bool populate_seekable = ! passive_p;
   unique_ptr pp (new sqlite_ps (internal_req_p ? db : dbq,
"rpm-seekable-query",
"select type, size, offset, mtime 
from " BUILDIDS "_r_seekable "
@@ -2749,6 +2750,9 @@ handle_buildid_r_match (bool internal_req_p,
 {
   if (rc != SQLITE_ROW)
 throw sqlite_exception(rc, "step");
+  // if we found a match in _r_seekable but we fail to extract it, don't
+  // bother populating it again
+  populate_seekable = false;
   const char* seekable_type = (const char*) sqlite3_column_text (*pp, 0);
   if (seekable_type != NULL && strcmp (seekable_type, "xz") == 0)
 {
@@ -2840,16 +2844,39 @@ handle_buildid_r_match (bool internal_req_p,
   throw archive_exception(a, "cannot open archive from pipe");
 }
 
-  // archive traversal is in three stages, no, four stages:
-  // 1) skip entries whose names do not match the requested one
-  // 2) extract the matching entry name (set r = result)
-  // 3) extract some number of prefetched entries (just into fdcache)
-  // 4) abort any further processing
+  // If the archive was scanned in a version without _r_seekable, then we may
+  // need to populate _r_seekable now.  This can be removed the next time
+  // BUILDIDS is updated.
+  if (populate_seekable)
+{
+  populate_seekable = is_seekable_archive (b_source0, a);
+  if (populate_seekable)
+{
+  // NB: the names are already interned
+  pp.reset(new sqlite_ps (db, "rpm-seekable-insert2",
+  "insert or ignore into " BUILDIDS 
"_r_seekable (file, content, type, size, offset, mtime) "
+  "values (?, "
+  "(select id from " BUILDIDS "_files "
+  "where dirname = (select id from " BUILDIDS 
"_fileparts where name = ?) "
+  "and basename = (select id from " BUILDIDS 
"_fileparts where name = ?) "
+  "), 'xz', ?, ?, ?)"));
+}
+}
+
+  // archive traversal is in five stages:
+  // 1) before we find a matching entry, insert it into _r_seekable if needed 
or
+  //skip it otherwise
+  // 2) extract the matching entry (set r = result).  Also insert it into
+  //_r_seekable if needed
+  // 3) extract some number of prefetched entries (just into fdcache).  Also
+  //insert them into _r_seekable if needed
+  // 4) if needed, insert all of the remaining entries into _r_seekable
+  // 5) abort any further processing
   struct MHD_Response* r = 0; // will set in stage 2
   unsigned prefetch_count =
 internal_req_p ? 0 : fdcache_prefetch;// will decrement in stage 3
 
-  while(r == 0 || prefetch_count > 0) // stage 1, 2, or 3
+  while(r == 0 || prefetch_count > 0 || populate_seekable) // stage 1-4
 {
   if (interrupted)
 break;
@@ -2863,6 +2890,43 @@ handle_buildid_r_match (bool internal_req_p,
 continue;
 
   string fn = canonicalized_archive_entry_pathname (e);
+
+  if (populate_seekable)
+{
+  string dn, bn;
+  size_t slash = fn.rfind('/');
+  if (slash == std::string::npos) {
+dn = "";
+bn = fn;
+  } else {
+dn = fn.substr(0, slash);
+bn = fn.substr(slash + 1);
+  }
+
+  int64_t seekable_size = archive_entry_size (e);
+  int64_t seekable_offset = archive_filter_bytes (a, 0);
+  time_t seekable_mtime = archive_entry_mtime (e);
+
+  pp->reset();
+  pp->bind(1, b_id0);
+  pp->bind(2, dn);
+  pp->bind(3, bn);
+  pp->bind(4, seekable_size);
+  pp->bind(5, seekable_offset);
+  pp->bind(6, seekable_mtime);
+  rc = pp->step();
+   

[PATCH v5 3/7] debuginfod: factor out common code for responding from an archive

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

handle_buildid_r_match has two very similar branches where it optionally
extracts a section and then creates a microhttpd response.  In
preparation for adding a third one, factor it out into a function.

Signed-off-by: Omar Sandoval 
---
 debuginfod/debuginfod.cxx | 213 +-
 1 file changed, 96 insertions(+), 117 deletions(-)

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 92022f3d..24702c23 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1965,6 +1965,81 @@ string canonicalized_archive_entry_pathname(struct 
archive_entry *e)
 }
 
 
+// NB: takes ownership of, and may reassign, fd.
+static struct MHD_Response*
+create_buildid_r_response (int64_t b_mtime0,
+   const string& b_source0,
+   const string& b_source1,
+   const string& section,
+   const string& ima_sig,
+   const char* tmppath,
+   int& fd,
+   off_t size,
+   time_t mtime,
+   const string& metric,
+   const struct timespec& extract_begin)
+{
+  if (tmppath != NULL)
+{
+  struct timespec extract_end;
+  clock_gettime (CLOCK_MONOTONIC, &extract_end);
+  double extract_time = (extract_end.tv_sec - extract_begin.tv_sec)
++ (extract_end.tv_nsec - extract_begin.tv_nsec)/1.e9;
+  fdcache.intern(b_source0, b_source1, tmppath, size, true, extract_time);
+}
+
+  if (!section.empty ())
+{
+  int scn_fd = extract_section (fd, b_mtime0,
+b_source0 + ":" + b_source1,
+section, extract_begin);
+  close (fd);
+  if (scn_fd >= 0)
+fd = scn_fd;
+  else
+{
+  if (verbose)
+obatched (clog) << "cannot find section " << section
+<< " for archive " << b_source0
+<< " file " << b_source1 << endl;
+  return 0;
+}
+
+  struct stat fs;
+  if (fstat (fd, &fs) < 0)
+{
+  close (fd);
+  throw libc_exception (errno,
+string ("fstat ") + b_source0 + string (" ") + section);
+}
+  size = fs.st_size;
+}
+
+  struct MHD_Response* r = MHD_create_response_from_fd (size, fd);
+  if (r == 0)
+{
+  if (verbose)
+obatched(clog) << "cannot create fd-response for " << b_source0 << 
endl;
+  close(fd);
+}
+  else
+{
+  inc_metric ("http_responses_total","result",metric);
+  add_mhd_response_header (r, "Content-Type", "application/octet-stream");
+  add_mhd_response_header (r, "X-DEBUGINFOD-SIZE", 
to_string(size).c_str());
+  add_mhd_response_header (r, "X-DEBUGINFOD-ARCHIVE", b_source0.c_str());
+  add_mhd_response_header (r, "X-DEBUGINFOD-FILE", b_source1.c_str());
+  if(!ima_sig.empty()) add_mhd_response_header(r, 
"X-DEBUGINFOD-IMASIGNATURE", ima_sig.c_str());
+  add_mhd_last_modified (r, mtime);
+  if (verbose > 1)
+obatched(clog) << "serving " << metric << " " << b_source0
+   << " file " << b_source1
+   << " section=" << section
+   << " IMA signature=" << ima_sig << endl;
+  /* libmicrohttpd will close fd. */
+}
+  return r;
+}
 
 static struct MHD_Response*
 handle_buildid_r_match (bool internal_req_p,
@@ -2142,57 +2217,15 @@ handle_buildid_r_match (bool internal_req_p,
   break; // branch out of if "loop", to try new libarchive fetch 
attempt
 }
 
-  if (!section.empty ())
-   {
- int scn_fd = extract_section (fd, fs.st_mtime,
-   b_source0 + ":" + b_source1,
-   section, extract_begin);
- close (fd);
- if (scn_fd >= 0)
-   fd = scn_fd;
- else
-   {
- if (verbose)
-   obatched (clog) << "cannot find section " << section
-   << " for archive " << b_source0
-   << " file " << b_source1 << endl;
- return 0;
-   }
-
- rc = fstat(fd, &fs);
- if (rc < 0)
-   {
- close (fd);
- throw libc_exception (errno,
-   string ("fstat archive ") + b_source0 + string (" file ") + 
b_source1
-   + string (" section ") + section);
-   }
-   }
-
-  struct MHD_Response* r = MHD_create_response_from_fd (fs.st_size, fd);
+  struct MHD_Response* r = create_buildid_r_response (b_mtime, b_source0,
+  b_source1, section,
+  ima_sig, NULL, fd,
+  f

[PATCH v5 6/7] debuginfod: populate _r_seekable on scan

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

Whenever a new archive is scanned, check if it is seekable with a little
liblzma magic, and populate _r_seekable if so.  With this, newly scanned
seekable archives will used the optimized extraction path added in the
previous commit.  Also add a test case using some artificial packages.

Signed-off-by: Omar Sandoval 
---
 debuginfod/debuginfod.cxx | 145 +-
 tests/Makefile.am |  13 +-
 ...pressme-seekable-xz-dbgsym_1.0-1_amd64.deb | Bin 0 -> 6288 bytes
 ...compressme-seekable-xz_1.0-1.debian.tar.xz | Bin 0 -> 1440 bytes
 .../compressme-seekable-xz_1.0-1.dsc  |  19 +++
 .../compressme-seekable-xz_1.0-1_amd64.deb| Bin 0 -> 6208 bytes
 .../compressme-seekable-xz_1.0.orig.tar.xz| Bin 0 -> 7160 bytes
 .../compressme-seekable-xz-1.0-1.src.rpm  | Bin 0 -> 15880 bytes
 .../compressme-seekable-xz-1.0-1.x86_64.rpm   | Bin 0 -> 31873 bytes
 ...sme-seekable-xz-debuginfo-1.0-1.x86_64.rpm | Bin 0 -> 21917 bytes
 ...e-seekable-xz-debugsource-1.0-1.x86_64.rpm | Bin 0 -> 7961 bytes
 tests/run-debuginfod-seekable.sh  | 144 +
 12 files changed, 316 insertions(+), 5 deletions(-)
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz-dbgsym_1.0-1_amd64.deb
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0-1.debian.tar.xz
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0-1.dsc
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0-1_amd64.deb
 create mode 100644 
tests/debuginfod-debs/seekable-xz/compressme-seekable-xz_1.0.orig.tar.xz
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-1.0-1.src.rpm
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-1.0-1.x86_64.rpm
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-debuginfo-1.0-1.x86_64.rpm
 create mode 100644 
tests/debuginfod-rpms/seekable-xz/compressme-seekable-xz-debugsource-1.0-1.x86_64.rpm
 create mode 100755 tests/run-debuginfod-seekable.sh

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index cf7f48ab..5fe2db0c 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1998,6 +1998,109 @@ struct lzma_exception: public reportable_exception
 //
 // 1: https://xz.tukaani.org/format/xz-file-format.txt
 
+// Return whether an archive supports seeking.
+static bool
+is_seekable_archive (const string& rps, struct archive* a)
+{
+  // Only xz supports seeking.
+  if (archive_filter_code (a, 0) != ARCHIVE_FILTER_XZ)
+return false;
+
+  int fd = open (rps.c_str(), O_RDONLY);
+  if (fd < 0)
+return false;
+  defer_dtor fd_closer (fd, close);
+
+  // Seek to the xz Stream Footer.  We assume that it's the last thing in the
+  // file, which is true for RPM and deb files.
+  off_t footer_pos = -LZMA_STREAM_HEADER_SIZE;
+  if (lseek (fd, footer_pos, SEEK_END) == -1)
+return false;
+
+  // Decode the Stream Footer.
+  uint8_t footer[LZMA_STREAM_HEADER_SIZE];
+  size_t footer_read = 0;
+  while (footer_read < sizeof (footer))
+{
+  ssize_t bytes_read = read (fd, footer + footer_read,
+ sizeof (footer) - footer_read);
+  if (bytes_read < 0)
+{
+  if (errno == EINTR)
+continue;
+  return false;
+}
+  if (bytes_read == 0)
+return false;
+  footer_read += bytes_read;
+}
+
+  lzma_stream_flags stream_flags;
+  lzma_ret ret = lzma_stream_footer_decode (&stream_flags, footer);
+  if (ret != LZMA_OK)
+return false;
+
+  // Seek to the xz Index.
+  if (lseek (fd, footer_pos - stream_flags.backward_size, SEEK_END) == -1)
+return false;
+
+  // Decode the Number of Records in the Index.  liblzma doesn't have an API 
for
+  // this if you don't want to decode the whole Index, so we have to do it
+  // ourselves.
+  //
+  // We need 1 byte for the Index Indicator plus 1-9 bytes for the
+  // variable-length integer Number of Records.
+  uint8_t index[10];
+  size_t index_read = 0;
+  while (index_read == 0) {
+  ssize_t bytes_read = read (fd, index, sizeof (index));
+  if (bytes_read < 0)
+{
+  if (errno == EINTR)
+continue;
+  return false;
+}
+  if (bytes_read == 0)
+return false;
+  index_read += bytes_read;
+  }
+  // The Index Indicator must be 0.
+  if (index[0] != 0)
+return false;
+
+  lzma_vli num_records;
+  size_t pos = 0;
+  size_t in_pos = 1;
+  while (true)
+{
+  if (in_pos >= index_read)
+{
+  ssize_t bytes_read = read (fd, index, sizeof (index));
+  if (bytes_read < 0)
+  {
+if (errno == EINTR)
+  continue;
+return false;
+  }
+  if (bytes_read == 0)
+return false;
+  index_read = bytes_read;
+  in_pos = 0;
+   

[PATCH v5 4/7] debugifod: add new table and views for seekable archives

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

In order to extract a file from a seekable archive, we need to know
where in the uncompressed archive the file data starts and its size.
Additionally, in order to populate the response headers, we need the
file modification time (since we won't be able to get it from the
archive metadata).  Add a new table, _r_seekable, keyed on the archive
file id and entry file id and containing the size, offset, and mtime.
It also contains the compression type just in case new seekable formats
are supported in the future.

In order to search this table when we get a request, we need the file
ids available.  Add the ids to the _query_d and _query_e views, and
rename them to _query_d2 and _query_e2.

This schema change is backward compatible and doesn't require
reindexing.  _query_d2 and _query_e2 can be renamed back the next time
BUILDIDS needs to be bumped.

Before this change, the database for a single kernel debuginfo RPM
(kernel-debuginfo-6.9.6-200.fc40.x86_64.rpm) was about 15MB.  This
change increases that by about 70kB, only a 0.5% increase.

Signed-off-by: Omar Sandoval 
---
 debuginfod/debuginfod.cxx | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 24702c23..b3d80090 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -265,25 +265,39 @@ static const char DEBUGINFOD_SQLITE_DDL[] =
   "foreign key (content) references " BUILDIDS "_files(id) on update 
cascade on delete cascade,\n"
   "primary key (content, file, mtime)\n"
   ") " WITHOUT_ROWID ";\n"
+  "create table if not exists " BUILDIDS "_r_seekable (\n" // seekable rpm 
contents
+  "file integer not null,\n"
+  "content integer not null,\n"
+  "type text not null,\n"
+  "size integer not null,\n"
+  "offset integer not null,\n"
+  "mtime integer not null,\n"
+  "foreign key (file) references " BUILDIDS "_files(id) on update 
cascade on delete cascade,\n"
+  "foreign key (content) references " BUILDIDS "_files(id) on update 
cascade on delete cascade,\n"
+  "primary key (file, content)\n"
+  ") " WITHOUT_ROWID ";\n"
   // create views to glue together some of the above tables, for webapi D 
queries
-  "create view if not exists " BUILDIDS "_query_d as \n"
+  // NB: _query_d2 and _query_e2 were added to replace _query_d and _query_e
+  // without updating BUILDIDS.  They can be renamed back the next time 
BUILDIDS
+  // is updated.
+  "create view if not exists " BUILDIDS "_query_d2 as \n"
   "select\n"
-  "b.hex as buildid, n.mtime, 'F' as sourcetype, f0.name as source0, 
n.mtime as mtime, null as source1\n"
+  "b.hex as buildid, 'F' as sourcetype, n.file as id0, f0.name as 
source0, n.mtime as mtime, null as id1, null as source1\n"
   "from " BUILDIDS "_buildids b, " BUILDIDS "_files_v f0, " BUILDIDS 
"_f_de n\n"
   "where b.id = n.buildid and f0.id = n.file and n.debuginfo_p = 1\n"
   "union all select\n"
-  "b.hex as buildid, n.mtime, 'R' as sourcetype, f0.name as source0, 
n.mtime as mtime, f1.name as source1\n"
+  "b.hex as buildid, 'R' as sourcetype, n.file as id0, f0.name as 
source0, n.mtime as mtime, n.content as id1, f1.name as source1\n"
   "from " BUILDIDS "_buildids b, " BUILDIDS "_files_v f0, " BUILDIDS 
"_files_v f1, " BUILDIDS "_r_de n\n"
   "where b.id = n.buildid and f0.id = n.file and f1.id = n.content and 
n.debuginfo_p = 1\n"
   ";"
   // ... and for E queries
-  "create view if not exists " BUILDIDS "_query_e as \n"
+  "create view if not exists " BUILDIDS "_query_e2 as \n"
   "select\n"
-  "b.hex as buildid, n.mtime, 'F' as sourcetype, f0.name as source0, 
n.mtime as mtime, null as source1\n"
+  "b.hex as buildid, 'F' as sourcetype, n.file as id0, f0.name as 
source0, n.mtime as mtime, null as id1, null as source1\n"
   "from " BUILDIDS "_buildids b, " BUILDIDS "_files_v f0, " BUILDIDS 
"_f_de n\n"
   "where b.id = n.buildid and f0.id = n.file and n.executable_p = 1\n"
   "union all select\n"
-  "b.hex as buildid, n.mtime, 'R' as sourcetype, f0.name as source0, 
n.mtime as mtime, f1.name as source1\n"
+  "b.hex as buildid, 'R' as sourcetype, n.file as id0, f0.name as 
source0, n.mtime as mtime, n.content as id1, f1.name as source1\n"
   "from " BUILDIDS "_buildids b, " BUILDIDS "_files_v f0, " BUILDIDS 
"_files_v f1, " BUILDIDS "_r_de n\n"
   "where b.id = n.buildid and f0.id = n.file and f1.id = n.content and 
n.executable_p = 1\n"
   ";"
@@ -2557,7 +2571,7 @@ handle_buildid (MHD_Connection* conn,
   if (atype_code == "D")
 {
   pp = new sqlite_ps (thisdb, "mhd-query-d",
-  "select mtime, sourcetype, source0, source1 from " 
BUILDIDS "_query_d where buildid = ? "
+  "select mtime, sourcetype, sour

[PATCH v5 5/7] debuginfod: optimize extraction from seekable xz archives

2024-07-23 Thread Omar Sandoval
From: Omar Sandoval 

The kernel debuginfo packages on Fedora, Debian, and Ubuntu, and many of
their downstreams, are all compressed with xz in multi-threaded mode,
which allows random access.  We can use this to bypass the full archive
extraction and dramatically speed up kernel debuginfo requests (from ~50
seconds in the worst case to < 0.25 seconds).

This works because multi-threaded xz compression splits up the stream
into many independently compressed blocks.  The stream ends with an
index of blocks.  So, to seek to an offset, we find the block containing
that offset in the index and then decompress and throw away data until
we reach the offset within the block.  We can then decompress the
desired amount of data, possibly from subsequent blocks.  There's no
high-level API in liblzma to do this, but we can do it by stitching
together a few low-level APIs.

We need to pass down the file ids then look up the size, uncompressed
offset, and mtime in the _r_seekable table.  Note that this table is not
yet populated, so this commit has no functional change on its own.

Signed-off-by: Omar Sandoval 
---
 configure.ac  |   5 +
 debuginfod/Makefile.am|   2 +-
 debuginfod/debuginfod.cxx | 456 +-
 3 files changed, 457 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 24e68d94..9c5f7e51 100644
--- a/configure.ac
+++ b/configure.ac
@@ -441,8 +441,13 @@ eu_ZIPLIB(bzlib,BZLIB,bz2,BZ2_bzdopen,bzip2)
 # We need this since bzip2 doesn't have a pkgconfig file.
 BZ2_LIB="$LIBS"
 AC_SUBST([BZ2_LIB])
+save_LIBS="$LIBS"
+LIBS=
 eu_ZIPLIB(lzma,LZMA,lzma,lzma_auto_decoder,[LZMA (xz)])
+lzma_LIBS="$LIBS"
+LIBS="$lzma_LIBS $save_LIBS"
 AS_IF([test "x$with_lzma" = xyes], [LIBLZMA="liblzma"], [LIBLZMA=""])
+AC_SUBST([lzma_LIBS])
 AC_SUBST([LIBLZMA])
 eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
 AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index b74e3673..e199dc0c 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -70,7 +70,7 @@ bin_PROGRAMS += debuginfod-find
 endif
 
 debuginfod_SOURCES = debuginfod.cxx
-debuginfod_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod) $(argp_LDADD) 
$(fts_LIBS) $(libmicrohttpd_LIBS) $(sqlite3_LIBS) $(libarchive_LIBS) 
$(rpm_LIBS) $(jsonc_LIBS) $(libcurl_LIBS) -lpthread -ldl
+debuginfod_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod) $(argp_LDADD) 
$(fts_LIBS) $(libmicrohttpd_LIBS) $(sqlite3_LIBS) $(libarchive_LIBS) 
$(rpm_LIBS) $(jsonc_LIBS) $(libcurl_LIBS) $(lzma_LIBS) -lpthread -ldl
 
 debuginfod_find_SOURCES = debuginfod-find.c
 debuginfod_find_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod) 
$(argp_LDADD) $(fts_LIBS) $(jsonc_LIBS)
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index b3d80090..cf7f48ab 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -63,6 +63,10 @@ extern "C" {
 #undef __attribute__ /* glibc bug - rhbz 1763325 */
 #endif
 
+#ifdef USE_LZMA
+#include 
+#endif
+
 #include 
 #include 
 #include 
@@ -1961,6 +1965,385 @@ handle_buildid_f_match (bool internal_req_t,
   return r;
 }
 
+
+#ifdef USE_LZMA
+struct lzma_exception: public reportable_exception
+{
+  lzma_exception(int rc, const string& msg):
+// liblzma doesn't have a lzma_ret -> string conversion function, so just
+// report the value.
+reportable_exception(string ("lzma error: ") + msg + ": error " + 
to_string(rc)) {
+  inc_metric("error_count","lzma",to_string(rc));
+}
+};
+
+// Neither RPM nor deb files support seeking to a specific file in the package.
+// Instead, to extract a specific file, we normally need to read the archive
+// sequentially until we find the file.  This is very slow for files at the end
+// of a large package with lots of files, like kernel debuginfo.
+//
+// However, if the compression format used in the archive supports seeking, we
+// can accelerate this.  As of July 2024, xz is the only widely-used format 
that
+// supports seeking, and usually only in multi-threaded mode.  Luckily, the
+// kernel-debuginfo package in Fedora and its downstreams, and the
+// linux-image-*-dbg package in Debian and its downstreams, all happen to use
+// this.
+//
+// The xz format [1] ends with an index of independently compressed blocks in
+// the stream.  In RPM and deb files, the xz stream is the last thing in the
+// file, so we assume that the xz Stream Footer is at the end of the package
+// file and do everything relative to that.  For each file in the archive, we
+// remember the size and offset of the file data in the uncompressed xz stream,
+// then we use the index to seek to that offset when we need that file.
+//
+// 1: https://xz.tukaani.org/format/xz-file-format.txt
+
+// Read the Index at the end of an xz file.
+static lzma_index*
+read_xz_index (int fd)
+{
+  off_t footer_pos = -LZMA_STREAM_HEADER_SIZE;
+  if (lseek (fd, footer