Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nexus-test-utils] set 60s timeouts for each init step #4789

Conversation

sunshowers
Copy link
Contributor

In #4779 we're tracking what appears to be a ClickHouse initialization failure
during Nexus startup. Set a timeout of 60s for each step in the initialization
process.

These steps should usually not take more than 5 seconds each, so 60s is a
really comfortable buffer.

Created using spr 1.3.5
Created using spr 1.3.5
Copy link
Collaborator

@smklein smklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, strongly in favor of the better visibility during test setup!

@@ -604,18 +636,21 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
dns_config_client.dns_config_put(&dns_config).await.expect(
"Failed to send initial DNS records to internal DNS server",
);
dns_config
self.dns_config = Some(dns_config);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to avoid a move between setup steps, yeah?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this pattern, we no longer have a way to return a value that the next step can use. This is the easiest way to work around that (and follows the pattern the rest of the builder uses anyway).

I ran into something similar with the update-engine and came up with the design to use step handles that can be passed around, but only resolved once steps start executing. For example,

let buf_list = download_handle.into_value(cx.token()).await;
let temp_dirs =
temp_dirs_handle.into_value(cx.token()).await;
. But that's trying to do a lot more than we need here.

nexus/test-utils/src/lib.rs Outdated Show resolved Hide resolved
Created using spr 1.3.5
@sunshowers sunshowers enabled auto-merge (squash) January 10, 2024 01:11
@sunshowers sunshowers merged commit 9eba46d into main Jan 10, 2024
21 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/nexus-test-utils-set-60s-timeouts-for-each-init-step branch January 10, 2024 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants