Skip to content

Commit

Permalink
Clean up start up timeouts
Browse files Browse the repository at this point in the history
Summary:
Replacing usage of BUCKD_STARTUP_TIMEOUT (defaults to 10s) * 9 with a new env var BUCKD_STARTUP_INIT_TIMEOUT which defaults to 90s.

Adding this so that when e2e tests set BUCKD_STARTUP_TIMEOUT to 120s, it doesn't get set to  120s * 9 which is longer than the test timeout of 600s and we lose information on where it timed out (in order to debug timeouts in D68655291).

Reviewed By: rajneesh

Differential Revision: D69068863

fbshipit-source-id: 55847a7cc2f979c3270ed706236069f1b1d2d766
  • Loading branch information
christolliday authored and facebook-github-bot committed Feb 4, 2025
1 parent 41841b4 commit db720c6
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 10 deletions.
19 changes: 9 additions & 10 deletions app/buck2_client_ctx/src/daemon/client/connect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,6 @@ pub enum BuckdConnectConstraints {
Constraints(DaemonConstraintsRequest),
}

fn buckd_startup_timeout_var() -> buck2_error::Result<Option<u64>> {
buck2_env!("BUCKD_STARTUP_TIMEOUT", type=u64)
}

async fn get_channel(
endpoint: ConnectionType,
change_to_parent_dir: bool,
Expand Down Expand Up @@ -272,7 +268,13 @@ pub async fn new_daemon_api_client(

pub fn buckd_startup_timeout() -> buck2_error::Result<Duration> {
Ok(Duration::from_secs(
buckd_startup_timeout_var()?.unwrap_or(10),
buck2_env!("BUCKD_STARTUP_TIMEOUT", type=u64)?.unwrap_or(10),
))
}

pub fn buckd_startup_init_timeout() -> buck2_error::Result<Duration> {
Ok(Duration::from_secs(
buck2_env!("BUCKD_STARTUP_INIT_TIMEOUT", type=u64)?.unwrap_or(90),
))
}

Expand Down Expand Up @@ -380,10 +382,7 @@ impl<'a> BuckdLifecycle<'a> {
daemon_startup_config: &DaemonStartupConfig,
) -> buck2_error::Result<()> {
let project_dir = self.paths.project_root();
let timeout_secs = Duration::from_secs(env::var("BUCKD_STARTUP_TIMEOUT").map_or(10, |t| {
t.parse::<u64>()
.unwrap_or_else(|_| panic!("Cannot convert {} to int", t))
}));
let timeout_secs = buckd_startup_timeout()?;

let daemon_exe = get_daemon_exe()?;
let slice_name = format!(
Expand Down Expand Up @@ -680,7 +679,7 @@ async fn establish_connection(
) -> buck2_error::Result<BootstrapBuckdClient> {
// There are many places where `establish_connection_inner` may hang.
// If it does, better print something to the user instead of hanging quietly forever.
let timeout = buckd_startup_timeout()? * 9;
let timeout = buckd_startup_init_timeout()?;
let deadline = StartupDeadline::duration_from_now(timeout)?;
deadline
.down(
Expand Down
1 change: 1 addition & 0 deletions tests/core/build/test_error_categorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ async def test_daemon_crash(buck: Buck, tmp_path: Path) -> None:

@buck_test()
@env("BUCKD_STARTUP_TIMEOUT", "0")
@env("BUCKD_STARTUP_INIT_TIMEOUT", "0")
async def test_connection_timeout(buck: Buck, tmp_path: Path) -> None:
record_path = tmp_path / "record.json"
res = await expect_failure(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ BUCK2_TEST_STDIN_BUFFER_SIZE usize
BUCK2_TEST_TOMBSTONED_DIGESTS HashSet<FileDigest>
BUCK2_TEST_TPX_USE_TCP bool false
BUCK2_WATCHMAN_TIMEOUT u64 57
BUCKD_STARTUP_INIT_TIMEOUT u64
BUCKD_STARTUP_TIMEOUT u64
BUCK_ACCESS_TIME_UPDATE_MAX_BUFFER_SIZE usize 5000
BUCK_CONSOLE String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ BUCK2_SCRIBE_CATEGORY String
BUCK2_TEST_BLOCK_ON_UPLOAD bool false
BUCK2_TEST_TPX_USE_TCP bool false
BUCK2_WATCHMAN_TIMEOUT u64 57
BUCKD_STARTUP_INIT_TIMEOUT u64
BUCKD_STARTUP_TIMEOUT u64
BUCK_ACCESS_TIME_UPDATE_MAX_BUFFER_SIZE usize 5000
BUCK_CONSOLE String
Expand Down
1 change: 1 addition & 0 deletions tests/e2e_util/buck_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ async def buck_fixture( # noqa C901 : "too complex"
# This is necessary for static linking on Linux.
if platform.system() != "Windows":
env["BUCKD_STARTUP_TIMEOUT"] = "120"
env["BUCKD_STARTUP_INIT_TIMEOUT"] = "120"

# allow_soft_errors will override any existing environment variable behavior
if marker.allow_soft_errors or marker.inplace:
Expand Down

0 comments on commit db720c6

Please sign in to comment.