Skip to content

Commit

Permalink
Support RE Engine tier/host/port override
Browse files Browse the repository at this point in the history
Summary:
Support overriding the execution engine address in the Buck config. This was supported in GRPC via the `engine_address` parameter but we didn't have the equivalent for Thrift.

Not reusing `engine_address` because I wanted to have separate address and port options; this maps better onto the underlying config struct.

Reviewed By: KapJI

Differential Revision: D66495632

fbshipit-source-id: 0d18754c2b1f07cbc381a2e0eb25c0fbf6e7a48d
  • Loading branch information
dougneal authored and facebook-github-bot committed Nov 27, 2024
1 parent 8913c00 commit de3b849
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
24 changes: 24 additions & 0 deletions app/buck2_execute/src/re/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,8 @@ impl RemoteExecutionClientImpl {
use remote_execution::ThreadConfig;

let mut re_client_config = create_default_config();

// gRPC settings
re_client_config.action_cache_client_config.connection_count =
static_metadata.action_cache_connection_count;
re_client_config.action_cache_client_config.address =
Expand Down Expand Up @@ -673,6 +675,28 @@ impl RemoteExecutionClientImpl {
.thrift_execution_client_config
.concurrency_limit = static_metadata.execution_concurrency_limit;

if let Some(engine_tier) = static_metadata.engine_tier.to_owned() {
re_client_config.thrift_execution_client_config.tier = engine_tier;
}

if static_metadata.engine_host.is_some() || static_metadata.engine_port.is_some() {
if static_metadata.engine_host.is_some()
&& static_metadata.engine_port.is_some()
{
re_client_config.thrift_execution_client_config.host_port =
Some(remote_execution::HostPort {
host: static_metadata.engine_host.clone().unwrap(),
port: static_metadata.engine_port.unwrap(),
..Default::default()
});
} else {
return Err(buck2_error!(
[],
"Both engine_host and engine_port must be set if either is set"
));
}
}

// TODO(ndmitchell): For now, we just drop RE log messages, but ideally we'd put them in our log stream.
let logger = slog::Logger::root(slog::Discard, slog::o!());
// TODO T179215751: If RE client fails we don't get the RE session ID and we can't find the RE logs.
Expand Down
21 changes: 20 additions & 1 deletion app/buck2_re_configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ mod fbcode {
/// Metadata that doesn't change between executions
#[derive(Clone, Debug, Default, Allocative)]
pub struct RemoteExecutionStaticMetadata {
// gRPC settings
pub cas_address: Option<String>,
pub cas_connection_count: i32,
pub cas_shared_cache: Option<String>,
Expand All @@ -44,7 +45,7 @@ mod fbcode {
pub action_cache_connection_count: i32,
pub engine_address: Option<String>,
pub engine_connection_count: i32,

// End gRPC settings
pub verbose_logging: bool,

pub use_manifold_rich_client: bool,
Expand Down Expand Up @@ -75,8 +76,14 @@ mod fbcode {

pub disable_fallocate: bool,
pub respect_file_symlinks: bool,

// Thrift settings
pub execute_over_thrift: bool,
pub execution_concurrency_limit: i32,
pub engine_tier: Option<String>,
pub engine_host: Option<String>,
pub engine_port: Option<i32>,
// End Thrift settings
}

impl RemoteExecutionStaticMetadataImpl for RemoteExecutionStaticMetadata {
Expand Down Expand Up @@ -238,6 +245,18 @@ mod fbcode {
property: "execution_concurrency_limit",
})?
.unwrap_or(4000),
engine_tier: legacy_config.parse(BuckconfigKeyRef {
section: BUCK2_RE_CLIENT_CFG_SECTION,
property: "engine_tier",
})?,
engine_host: legacy_config.parse(BuckconfigKeyRef {
section: BUCK2_RE_CLIENT_CFG_SECTION,
property: "engine_host",
})?,
engine_port: legacy_config.parse(BuckconfigKeyRef {
section: BUCK2_RE_CLIENT_CFG_SECTION,
property: "engine_port",
})?,
})
}

Expand Down

0 comments on commit de3b849

Please sign in to comment.