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

Daniel Thornburgh via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 7 14:38:00 PST 2023


================
@@ -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, "");
----------------
mysterymath wrote:

Reusing the file cache for this will inherit whatever cache pruning policy is configured for debuginfod files. It seems prima facie pretty unlikely that sharing this policy is desirable; looking at the defaults, it takes a week for entries to expire, and the cache is only pruned once every 20 minutes. This means that a momentary connection failure would become one of a much much greater scope.

I'm not convinced that negative caching is the way to go here; looking broadly around, it seems rare to cache negative HTTP requests, while negative DNS requests are already typically cached and should fail quickly.

There's also already `DEBUGINFOD_TIMEOUT`; it looks like we didn't quite implement this correctly. It should be the timeout to receive 100K, so there shouldn't be any harm in setting it very short if desired. We used `CURLOPT_TIMEOUT_MS` instead of `CURLOPT_LOW_SPEED_LIMIT` and `CURLOPT_LOW_SPEED_TIME`, which is what both libdebuginfod's docs and implementation suggest. Fixing this seems like a better way to address this, since it doesn't require inventing any new mechanism.

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


More information about the llvm-commits mailing list