-
Notifications
You must be signed in to change notification settings - Fork 834
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
cumulus: bump PARENT_SEARCH_DEPTH and add test for 12-core elastic scaling #6983
Conversation
…dima/zombienet-sdk-rewrite
…into alindima/update-parent-search-depth
…into alindima/update-parent-search-depth
polkadot/zombienet-sdk-tests/tests/elastic_scaling/slot_based_12cores.rs
Show resolved
Hide resolved
cumulus/test/runtime/src/lib.rs
Outdated
#[cfg(feature = "elastic-scaling-500ms")] | ||
const UNINCLUDED_SEGMENT_CAPACITY: u32 = 30; | ||
#[cfg(feature = "elastic-scaling-500ms")] | ||
const BLOCK_PROCESSING_VELOCITY: u32 = 12; | ||
#[cfg(feature = "elastic-scaling-500ms")] | ||
pub const MILLISECS_PER_BLOCK: u64 = 500; |
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.
Can we just define MILLISECS_PER_BLOCK
and then compute the BLOCK_PROCESSING_VELOCITY
and UNINCLUDED_SEGMENT_CAPACITY
based on it ? It would also reduce the amount cluttering due to feature flags.
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.
good idea, done
All GitHub workflows were cancelled due to failure one of the required jobs. |
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.
LGTM! Nice!
* master: (58 commits) [pallet-revive] pack exceeding syscall arguments into registers (#7319) cumulus: bump PARENT_SEARCH_DEPTH and add test for 12-core elastic scaling (#6983) xcm: fix for DenyThenTry Barrier (#7169) Migrating polkadot-runtime-common slots benchmarking to v2 (#6614) Add development chain-spec file for minimal/parachain templates for Omni Node compatibility (#6529) `Arc` definition in `TransactionPool` (#7042) [sync] Let new subscribers know about already connected peers (backward-compatible) (#7344) Removed unused dependencies (partial progress) (#7329) Improve debugging by using `#[track_caller]` in system `assert_last_event` and `assert_has_event` (#7142) `set_validation_data` register weight manually, do not use refund when the pre dispatch is zero. (#7327) Fix the link to the chain snapshots (#7330) revive: Fix compilation of `uapi` crate when `unstable-hostfn` is not set (#7318) [pallet-revive] eth-rpc minor fixes (#7325) sync-templates: enable syncing from stable release patches (#7227) Bridges: emulated tests small nits/improvements (#7322) fix(cmd bench-omni): build omni-bencher with production profile (#7299) Nits for collectives-westend XCM benchmarks setup (#7215) bench all weekly - and fix for pallet_multisig lib (#6789) Deprecate ParaBackingState API (#6867) Fix setting the image properly (#7315) ...
On top of #6757
Fixes #6858 by bumping the
PARENT_SEARCH_DEPTH
constant to a larger value (30) and adds a zombienet-sdk test that exercises the 12-core scenario.This is a node-side limit that restricts the number of allowed pending availability candidates when choosing the parent parablock during authoring.
This limit is rather redundant, as the parachain runtime already restricts the unincluded segment length to the configured value in the FixedVelocityConsensusHook (which ideally should be equal to this
PARENT_SEARCH_DEPTH
).For 12 cores, a value of 24 should be enough, but I bumped it to 30 to have some extra buffer.
There are two other potential ways of fixing this: