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

subsystem-bench: adjust test config to Kusama #3583

Merged
merged 7 commits into from
Mar 11, 2024

Conversation

AndreiEres
Copy link
Contributor

@AndreiEres AndreiEres commented Mar 5, 2024

Fixes #3528

latency:
    mean_latency_ms = 30 // common sense
    std_dev = 2.0 // common sense
n_validators = 300 // max number of validators, from chain config
n_cores = 60 // 300/5
max_validators_per_core = 5 // default
min_pov_size = 5120 // max
max_pov_size = 5120 // max
peer_bandwidth = 44040192 // from the Parity's kusama validators
bandwidth = 44040192 // from the Parity's kusama validators
connectivity = 90 // we need to be connected to 90-95% of peers

@AndreiEres AndreiEres added R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights. labels Mar 5, 2024
@AndreiEres
Copy link
Contributor Author

AndreiEres commented Mar 5, 2024

image

source

We could've used these values assuming 90 and 99 percentiles

min_pov_size = 64 // 108 KiB, 90%
max_pov_size = 2024 // 1.78 MiB, 99%

fn default_pov_size() -> usize {
5120
2 * 1024
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use max value here such that any degradation has more impact.

fn default_backing_group_size() -> usize {
5
3
Copy link
Contributor

Choose a reason for hiding this comment

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

This has to be kept in sync with the actual value from chain. Let's keep it 5 here and override in specific benchmarks.

300
}

// Based on number of kusama validators producing blocks
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not related to the validators producing blocks. It's the total number of cores: cores assigned to specific paras plus on-demand (chain config value)

Copy link
Contributor

Choose a reason for hiding this comment

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

But we don't want to fetch all cores. We'd want to represent a single node effort. Roughly this number has to be set to n_cores * needed_approvals / n_validators.

Copy link
Contributor

Choose a reason for hiding this comment

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

n_cores * needed_approvals / n_validators. only so for availability read, for write, we need to cover all cores as said above.

@AndreiEres AndreiEres marked this pull request as ready for review March 6, 2024 13:55
@AndreiEres AndreiEres added this pull request to the merge queue Mar 11, 2024
Merged via the queue into master with commit 05381af Mar 11, 2024
131 of 132 checks passed
@AndreiEres AndreiEres deleted the AndreiEres/regression-config branch March 11, 2024 16:50
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Mar 24, 2024
Fixes paritytech#3528

```rust
latency:
    mean_latency_ms = 30 // common sense
    std_dev = 2.0 // common sense
n_validators = 300 // max number of validators, from chain config
n_cores = 60 // 300/5
max_validators_per_core = 5 // default
min_pov_size = 5120 // max
max_pov_size = 5120 // max
peer_bandwidth = 44040192 // from the Parity's kusama validators
bandwidth = 44040192 // from the Parity's kusama validators
connectivity = 90 // we need to be connected to 90-95% of peers
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust configuration in subsystem regression tests to Kusama network
3 participants