Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
Change to not upload artifacts when fetch result is cache error
Browse files Browse the repository at this point in the history
Summary:
This change intends to make ARL better working with BuckCache. Currently ARL returns a 503 error to client, where the client considers this as a [cache error](https://www.internalfb.com/code/fbsource/[a2628e6000497e1478a622d2d1ee3a6b0ca7c362]/xplat/build_infra/buck_client/src/com/facebook/buck/artifact_cache/ThriftArtifactCache.java?lines=480). When cache error happens (which indicates the servers are overloaded), we don't want to create more upload traffic to overwhelm more servers.
To see a general metrics about cache errors happening nowadays in production: https://fburl.com/scuba/buck_builds/5ax2wm71

Reviewed By: lebentle

fbshipit-source-id: 597216f2b17ec98eb71035d2afff2fa6f1bbf1cc
  • Loading branch information
Lydia Yu authored and facebook-github-bot committed Apr 19, 2022
1 parent 4949fbf commit 4d3809b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import com.facebook.buck.artifact_cache.ArtifactCache;
import com.facebook.buck.artifact_cache.ArtifactUploader;
import com.facebook.buck.artifact_cache.CacheResult;
import com.facebook.buck.artifact_cache.CacheResultType;
import com.facebook.buck.core.build.engine.BuildRuleSuccessType;
import com.facebook.buck.core.build.engine.buildinfo.BuildInfo;
import com.facebook.buck.core.build.engine.buildinfo.OnDiskBuildInfo;
Expand Down Expand Up @@ -193,13 +195,18 @@ public ListenableFuture<Unit> uploadToCache(BuildRuleSuccessType success, long b

/** @return whether we should upload the given rules artifacts to cache. */
public UploadToCacheResultType shouldUploadToCache(
BuildRuleSuccessType successType, long outputSize) {
BuildRuleSuccessType successType, Optional<CacheResult> cacheResult, long outputSize) {

// The success type must allow cache uploading.
if (!successType.shouldUploadResultingArtifact()) {
return UploadToCacheResultType.UNCACHEABLE;
}

// If we failed to fetch with error, we won't upload again
if (cacheResult.isPresent() && cacheResult.get().getType() == CacheResultType.ERROR) {
return UploadToCacheResultType.UNCACHEABLE_DUE_TO_CACHE_ERROR;
}

// The cache must be writable.
if (!artifactCache.getCacheReadMode().isWritable()) {
return UploadToCacheResultType.CACHEABLE_READONLY_CACHE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -874,8 +874,9 @@ private void handleResult(BuildResult input) {
outputHash = hashString.getLeftOption().map(HashCode::fromString);

// Determine if this is rule is cacheable.
Optional<CacheResult> cacheResult = input.getCacheResult();
shouldUploadToCache =
buildCacheArtifactUploader.shouldUploadToCache(success, outputSizeValue);
buildCacheArtifactUploader.shouldUploadToCache(success, cacheResult, outputSizeValue);

// Upload it to the cache.
if (shouldUploadToCache.equals(UploadToCacheResultType.CACHEABLE)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ public enum UploadToCacheResultType {
// The specific rule has been marked as un-cacheable.
UNCACHEABLE_RULE,

// The rule is un-cacheable due to cache error, which might caused by server error
UNCACHEABLE_DUE_TO_CACHE_ERROR,

// The rule could have been cached but was over the allowed output_size for the cache.
CACHEABLE_OVER_SIZE_LIMIT,

Expand Down

0 comments on commit 4d3809b

Please sign in to comment.