Skip to content

Commit c5c807a

Browse files
cormacrelffacebook-github-bot
authored andcommitted
buck2_execute: implement OSS upload_blob for local_only cache uploads (#765)
Summary: Forward-port of patch 4 in <#477>, providing a clear piece of missing functionality: in the event that stdout or stderr were more than 50KiB of output when caching `local_only` actions, then this dead path was taken, and so stdout/stderr would not be uploaded successfully in the cache. Pull Request resolved: #765 Reviewed By: ndmitchell, rajneesh Differential Revision: D66988951 Pulled By: IanChilds fbshipit-source-id: a9b53eacb16af3ed50df41405842138dcb2d3f83
1 parent 53f4592 commit c5c807a

File tree

5 files changed

+38
-13
lines changed

5 files changed

+38
-13
lines changed

app/buck2_execute/src/execute/output.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ use buck2_common::file_ops::FileDigest;
1414
use buck2_core::execution_types::executor_config::RemoteExecutorUseCase;
1515
use buck2_error::BuckErrorContext;
1616
use futures::future;
17+
use remote_execution::InlinedBlobWithDigest;
1718
use remote_execution::TDigest;
1819

1920
use crate::digest::CasDigestConversionResultExt;
2021
use crate::digest::CasDigestFromReExt;
22+
use crate::digest::CasDigestToReExt;
2123
use crate::digest_config::DigestConfig;
2224
use crate::re::manager::ManagedRemoteExecutionClient;
2325
use crate::re::streams::RemoteCommandStdStreams;
@@ -238,12 +240,13 @@ impl CommandStdStreams {
238240
self,
239241
client: &ManagedRemoteExecutionClient,
240242
use_case: RemoteExecutorUseCase,
243+
digest_config: DigestConfig,
241244
) -> buck2_error::Result<StdStreamPair<ReStdStream>> {
242245
match self {
243246
Self::Local { stdout, stderr } => {
244247
let (stdout, stderr) = future::try_join(
245-
maybe_upload_to_re(client, use_case, stdout),
246-
maybe_upload_to_re(client, use_case, stderr),
248+
maybe_upload_to_re(client, use_case, stdout, digest_config),
249+
maybe_upload_to_re(client, use_case, stderr, digest_config),
247250
)
248251
.await?;
249252

@@ -277,11 +280,17 @@ async fn maybe_upload_to_re(
277280
client: &ManagedRemoteExecutionClient,
278281
use_case: RemoteExecutorUseCase,
279282
bytes: Vec<u8>,
283+
digest_config: DigestConfig,
280284
) -> buck2_error::Result<ReStdStream> {
281285
const MIN_STREAM_UPLOAD_SIZE: usize = 50 * 1024; // Same as RE
282286
if bytes.len() < MIN_STREAM_UPLOAD_SIZE {
283287
return Ok(ReStdStream::Raw(bytes));
284288
}
285-
let digest = client.upload_blob(bytes, use_case).await?;
289+
let inline_blob = InlinedBlobWithDigest {
290+
digest: FileDigest::from_content(&bytes, digest_config.cas_digest_config()).to_re(),
291+
blob: bytes,
292+
..Default::default()
293+
};
294+
let digest = client.upload_blob(inline_blob, use_case).await?;
286295
Ok(ReStdStream::Digest(digest))
287296
}

app/buck2_execute/src/re/client.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ impl RemoteExecutionClient {
303303

304304
pub async fn upload_blob(
305305
&self,
306-
blob: Vec<u8>,
306+
blob: InlinedBlobWithDigest,
307307
use_case: RemoteExecutorUseCase,
308308
) -> buck2_error::Result<TDigest> {
309309
self.data
@@ -1228,14 +1228,14 @@ impl RemoteExecutionClientImpl {
12281228

12291229
pub async fn upload_blob(
12301230
&self,
1231-
blob: Vec<u8>,
1231+
blob: InlinedBlobWithDigest,
12321232
use_case: RemoteExecutorUseCase,
12331233
) -> buck2_error::Result<TDigest> {
12341234
with_error_handler(
12351235
"upload_blob",
12361236
self.get_session_id(),
12371237
self.client()
1238-
.upload_blob(blob, use_case.metadata(None))
1238+
.upload_blob_with_digest(blob.blob, blob.digest, use_case.metadata(None))
12391239
.await,
12401240
)
12411241
.await

app/buck2_execute/src/re/manager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ impl ManagedRemoteExecutionClient {
471471

472472
pub async fn upload_blob(
473473
&self,
474-
blob: Vec<u8>,
474+
blob: InlinedBlobWithDigest,
475475
use_case: RemoteExecutorUseCase,
476476
) -> buck2_error::Result<TDigest> {
477477
let use_case = self.re_use_case_override.unwrap_or(use_case);

app/buck2_execute_impl/src/executors/caching.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ impl CacheUploader {
417417
.report
418418
.std_streams
419419
.clone()
420-
.into_re(&self.re_client, self.re_use_case)
420+
.into_re(&self.re_client, self.re_use_case, digest_config)
421421
.await
422422
.buck_error_context("Error accessing std_streams")
423423
};

remote_execution/oss/re_grpc/src/client.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -760,13 +760,29 @@ impl REClient {
760760
.await
761761
}
762762

763-
pub async fn upload_blob(
763+
pub async fn upload_blob_with_digest(
764764
&self,
765-
_blob: Vec<u8>,
766-
_metadata: RemoteExecutionMetadata,
765+
blob: Vec<u8>,
766+
digest: TDigest,
767+
metadata: RemoteExecutionMetadata,
767768
) -> anyhow::Result<TDigest> {
768-
// TODO(aloiscochard)
769-
Err(anyhow::anyhow!("Not implemented (RE upload_blob)"))
769+
let blob = InlinedBlobWithDigest {
770+
digest: digest.clone(),
771+
blob,
772+
..Default::default()
773+
};
774+
self.upload(
775+
metadata,
776+
UploadRequest {
777+
inlined_blobs_with_digest: Some(vec![blob]),
778+
files_with_digest: None,
779+
directories: None,
780+
upload_only_missing: false,
781+
..Default::default()
782+
},
783+
)
784+
.await?;
785+
Ok(digest)
770786
}
771787

772788
pub async fn download(

0 commit comments

Comments
 (0)