[llvm] Debuginfod failed-server cache (PR #74757)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Kevin Frei (kevinfrei)

<details>
<summary>Changes</summary>

When using the Debuginfod library to pull symbols, a non-responding server is a major performance hit. I've added a "failed server" cache so that if a server has failed to respond, the library won't continue to query it. The performance hit is moderately catastrophic in my experience: 'target create /bin/ls' takes 45 seconds to fail on a system that's trying to talk to a server that is unreachable.

I'll be exposing more configuration settings for use of Debuginfod in the very near term, but for right now, this eliminates a pretty major negative user experience for folks on a Linux system sitting behind a proxy server, or in similar network-constrained situations.

---
Full diff: https://github.com/llvm/llvm-project/pull/74757.diff


1 Files Affected:

- (modified) llvm/lib/Debuginfod/Debuginfod.cpp (+35-1) 


``````````diff
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 9df30ab55cbad..576cb8149cbb6 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -194,6 +194,26 @@ Error StreamedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) {
   return Error::success();
 }
 
+// Create a file cache entry to remember if a given server failed
+Expected<AddStreamFn> CheckServerFailure(FileCache &Cache, uint &Task,
+                                         StringRef ServerUrl) {
+  SmallString<96> CachedServerFailurePath(ServerUrl);
+  llvm::transform(CachedServerFailurePath, CachedServerFailurePath.begin(),
+                  [&](char c) { return std::isalnum(c) ? c : '_'; });
+  CachedServerFailurePath.append(".failed");
+  return Cache(Task, CachedServerFailurePath, "");
+}
+
+void RegisterFailedServer(AddStreamFn &ServerFailureFn, uint &Task) {
+  Expected<std::unique_ptr<CachedFileStream>> FileStreamOrError =
+      ServerFailureFn(Task, "");
+  if (!FileStreamOrError)
+    consumeError(FileStreamOrError.takeError());
+  else
+    // Create the server-failure file
+    *FileStreamOrError.get()->OS << "";
+}
+
 // An over-accepting simplification of the HTTP RFC 7230 spec.
 static bool isHeader(StringRef S) {
   StringRef Name;
@@ -269,6 +289,17 @@ Expected<std::string> getCachedOrDownloadArtifact(
   HTTPClient Client;
   Client.setTimeout(Timeout);
   for (StringRef ServerUrl : DebuginfodUrls) {
+    // First, check to make sure we should keep asking this server.
+    Expected<AddStreamFn> ServerFailureFnOrErr =
+        CheckServerFailure(Cache, Task, ServerUrl);
+    if (!ServerFailureFnOrErr)
+      return ServerFailureFnOrErr.takeError();
+    AddStreamFn &ServerFailureFn = *ServerFailureFnOrErr;
+    if (!ServerFailureFn)
+      // We found a 'server failure' file in the cache which means
+      // this server has failed before: don't bother trying it again.
+      continue;
+
     SmallString<64> ArtifactUrl;
     sys::path::append(ArtifactUrl, sys::path::Style::posix, ServerUrl, UrlPath);
 
@@ -280,8 +311,11 @@ Expected<std::string> getCachedOrDownloadArtifact(
       HTTPRequest Request(ArtifactUrl);
       Request.Headers = getHeaders();
       Error Err = Client.perform(Request, Handler);
-      if (Err)
+      if (Err) {
+        // Put a server-failure marker in the cache so we don't keep trying it.
+        RegisterFailedServer(ServerFailureFn, Task);
         return std::move(Err);
+      }
 
       unsigned Code = Client.responseCode();
       if (Code && Code != 200)

``````````

</details>


https://github.com/llvm/llvm-project/pull/74757