feat: make an option to set the max_number_of_canisters in test net#9704
feat: make an option to set the max_number_of_canisters in test net#9704shilingwang wants to merge 7 commits intomasterfrom
Conversation
…to install infinite number of canisters for load testing
Resolve add/add conflict in sev_baremetal_app_subnet.rs: keep Some(0) for max_number_of_canisters (unlimited) for SEV bare-metal load testing. Made-with: Cursor
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
| /// * `max_number_of_canisters` - Cap on canisters for the new subnet. `None` uses | ||
| /// the historical default of 4. `Some(0)` means no limit (same as ic-prep; see | ||
| /// `SubnetRecord` / canister_manager). |
There was a problem hiding this comment.
I wonder if it wouldn't be simpler to let the caller always specify the max_number_of_canisters instead of having this historical default of 4 hardcoded here.
Alternatively would there be a problem with just hardcoding this to 0 (no-limit)?
| /// * `replica_version` - Replica software version to install to the new subnet | ||
| /// nodes (see `get_software_version`) | ||
| /// * `max_number_of_canisters` - Cap on canisters for the new subnet. `None` uses | ||
| /// the historical default of 4. `Some(0)` means no limit (same as ic-prep; see |
There was a problem hiding this comment.
Why is the default 4? That seems like an awfully small number. I would set it to 0 by default, which also is what a testnet subnet that is not created via proposal will end up with. If there are existing tests that require such a small number, then they can set it themselves.
BTW 0 does not mean unlimited, it means whatever the replica decides as its default (120'000).
4 participants
We recently add new testnet which has a baremetal machine. This testnet is used for load testing, which has scenarios which many canisters are installed.