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

[Kevin Frei via llvm-commits]

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

>From 196eb334507c99e989f2ff476dc2359022096a66 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 6 Dec 2023 16:41:43 -0800
Subject: [PATCH 1/2] Added failed server caching to Debuginfod

---
 llvm/lib/Debuginfod/Debuginfod.cpp | 36 +++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 9df30ab55cbad4..53195f59148916 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -194,6 +194,20 @@ Error StreamedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) {
   return Error::success();
 }
 
+// Make a file 'cache' to remember if a given server failed
+Expected<AddStreamFn> GetServerFailureCache(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, "");
+}
+
+bool ShouldSkipServer(StringRef ServerID) { return true; }
+
+void RegisterFailedServer(StringRef ServerID) { return; }
+
 // An over-accepting simplification of the HTTP RFC 7230 spec.
 static bool isHeader(StringRef S) {
   StringRef Name;
@@ -269,6 +283,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> ServerFailureCacheOrErr =
+        GetServerFailureCache(Cache, Task, ServerUrl);
+    if (!ServerFailureCacheOrErr)
+      return ServerFailureCacheOrErr.takeError();
+    AddStreamFn &ServerFailureCache = *ServerFailureCacheOrErr;
+    if (!ServerFailureCache)
+      // We found a 'server failure' file in 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 +305,17 @@ Expected<std::string> getCachedOrDownloadArtifact(
       HTTPRequest Request(ArtifactUrl);
       Request.Headers = getHeaders();
       Error Err = Client.perform(Request, Handler);
-      if (Err)
+      if (Err) {
+        // Put a server-failuire marker in the cache so we don't keep trying it
+        Expected<std::unique_ptr<CachedFileStream>> FileStreamOrError =
+            ServerFailureCache(Task, "");
+        if (!FileStreamOrError)
+          consumeError(FileStreamOrError.takeError());
+        else
+          // Create the server-failure file
+          *FileStreamOrError.get()->OS << "";
         return std::move(Err);
+      }
 
       unsigned Code = Client.responseCode();
       if (Code && Code != 200)

>From 63a4182a8f098386aefa7cac2ef2556b4d5000f0 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Thu, 7 Dec 2023 11:54:38 -0800
Subject: [PATCH 2/2] Final cleanup: prep for public PR

---
 llvm/lib/Debuginfod/Debuginfod.cpp | 42 +++++++++++++++---------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index 53195f59148916..576cb8149cbb6b 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -194,9 +194,9 @@ Error StreamedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) {
   return Error::success();
 }
 
-// Make a file 'cache' to remember if a given server failed
-Expected<AddStreamFn> GetServerFailureCache(FileCache &Cache, uint &Task,
-                                            StringRef ServerUrl) {
+// 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 : '_'; });
@@ -204,9 +204,15 @@ Expected<AddStreamFn> GetServerFailureCache(FileCache &Cache, uint &Task,
   return Cache(Task, CachedServerFailurePath, "");
 }
 
-bool ShouldSkipServer(StringRef ServerID) { return true; }
-
-void RegisterFailedServer(StringRef ServerID) { return; }
+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) {
@@ -284,13 +290,13 @@ Expected<std::string> getCachedOrDownloadArtifact(
   Client.setTimeout(Timeout);
   for (StringRef ServerUrl : DebuginfodUrls) {
     // First, check to make sure we should keep asking this server.
-    Expected<AddStreamFn> ServerFailureCacheOrErr =
-        GetServerFailureCache(Cache, Task, ServerUrl);
-    if (!ServerFailureCacheOrErr)
-      return ServerFailureCacheOrErr.takeError();
-    AddStreamFn &ServerFailureCache = *ServerFailureCacheOrErr;
-    if (!ServerFailureCache)
-      // We found a 'server failure' file in cache which means
+    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;
 
@@ -306,14 +312,8 @@ Expected<std::string> getCachedOrDownloadArtifact(
       Request.Headers = getHeaders();
       Error Err = Client.perform(Request, Handler);
       if (Err) {
-        // Put a server-failuire marker in the cache so we don't keep trying it
-        Expected<std::unique_ptr<CachedFileStream>> FileStreamOrError =
-            ServerFailureCache(Task, "");
-        if (!FileStreamOrError)
-          consumeError(FileStreamOrError.takeError());
-        else
-          // Create the server-failure file
-          *FileStreamOrError.get()->OS << "";
+        // Put a server-failure marker in the cache so we don't keep trying it.
+        RegisterFailedServer(ServerFailureFn, Task);
         return std::move(Err);
       }