-
Notifications
You must be signed in to change notification settings - Fork 44
Increase Canton startup timeouts #2361
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR increases the Canton container startup timeout from 5 minutes to 1 hour to accommodate varying infrastructure performance during test execution.
Changes:
- Updated startup timeout for Canton containers to 1 hour
- Added necessary time package import to canton.go
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| framework/components/blockchain/canton/splice.go | Updated startup timeout from 5 minutes to 1 hour |
| framework/components/blockchain/canton/canton.go | Added time import and set startup timeout to 1 hour |
| framework/.changeset/v0.13.9.md | Added changelog entry for the timeout increase |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| - Increase Canton startup timeouts to one hour No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious, why do they need so much time to start? 5 minutes already sounds like quite a lot. Won't that result for example in CI hanging for 1h if startup fails for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting up the containers, especially the Splice one it really resource intense. Under normal circumstances it really shouldn't take this long and should be in the range of ~2-3 minutes but it ultimately depends on the runner that this runs on.
Whoever calls this should definitely provide a timeout, e.g. Go's default 10m test timeout, but we shouldn't enforce any specific value from within CTF.
Removing the WithStartupTimeout will lead to testcontainers using a default of 60s, therefore setting it to 1h which seems like a somewhat sane value that should never be hit.
This effectively removes the startup timeout for the Canton containers by setting the timeout to
1 hour. Depending on the infra that tests run on, the time required for startup can vary wildly, making it hard to predict.Callers should use the context to time out if required.
Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.
Why
The pull request extends startup timeouts for Canton and Splice containers in the testing framework to one hour. This change aims to mitigate issues related to container readiness checks in environments where the startup might be slower than expected.
What
ContainerRequestfunction for Canton by setting the startup timeout to one hour. This adjustment aims to give the Canton container sufficient time to start up, especially in resource-constrained environments.SpliceContainerRequestfunction's startup timeout to one hour, aligning it with the changes made to the Canton container. This ensures that both components have adequate time to initialize before the system performs readiness checks.