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

Run benchmarks and update weights for pallet_collator_selection once benchmarks are fixed for external pallets #608

Closed
4 tasks done
shannonwells opened this issue Nov 1, 2022 · 4 comments · Fixed by #985
Assignees
Labels
chore No feature changes tech debt

Comments

@shannonwells
Copy link
Collaborator

shannonwells commented Nov 1, 2022

Update benchmarks to use the proper local Benchmarks when this is working

Todo

  • Add it to the list of benchmarks to run scripts/run_all_benchmarks.sh
  • Add the generated file to runtime/common/src/weights/mod.rs
  • Run the new benchmarks on the reference hardware
@wilwade wilwade added this to the Upgrade to v0.9.31 milestone Nov 4, 2022
@wilwade wilwade added the blocked Blocked by another issue label Nov 14, 2022
@wilwade wilwade changed the title Run benchmarks and update weights once benchmarks are fixed for external pallets Run benchmarks and update weights for pallet_collator_selection once benchmarks are fixed for external pallets Nov 17, 2022
@wilwade wilwade added the planning Discuss & point in planning meeting label Nov 21, 2022
@saraswatpuneet saraswatpuneet added chore No feature changes and removed planning Discuss & point in planning meeting labels Nov 28, 2022
@saraswatpuneet saraswatpuneet removed the blocked Blocked by another issue label Dec 13, 2022
@p5150j p5150j removed this from the Upgrade to v0.9.31 milestone Jan 4, 2023
@p5150j
Copy link
Collaborator

p5150j commented Jan 20, 2023

on hold, revisit after 0.9.36 upgrade

@rlaferla
Copy link
Contributor

rlaferla commented Jan 26, 2023

Even with 0.9.36, the benchmarks fail. I am researching the error that is causing unwrap() to panic.

2023-01-26 12:57:20 Starting benchmark: pallet_collator_selection::register_as_candidate    
2023-01-26 12:57:20 panicked at 'called `Result::unwrap()` on an `Err` value: <wasm:stripped>', /workarea/.cargo/git/checkouts/cumulus-59522f43471fa161/afe528a/pallets/collator-selection/src/benchmarking.rs:103:86    
Error: Input("Error executing and verifying runtime benchmark: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\n\n    0: 0x38c7 - <unknown>!rust_begin_unwind\n    1: 0x1b3e - <unknown>!core::panicking::panic_fmt::h57b56b1dc717ba48\n    2: 0x2208 - <unknown>!core::result::unwrap_failed::h4020e4ec86ef2ed8\n    3: 0x16c137 - <unknown>!pallet_collator_selection::benchmarking::register_candidates::hbdbf72fa3adb997a\n    4: 0x1488d6 - <unknown>!<frequency_runtime::Runtime as frame_benchmarking::utils::runtime_decl_for_Benchmark::BenchmarkV1<sp_runtime::generic::block::Block<sp_runtime::generic::header::Header<u32,sp_runtime::traits::BlakeTwo256>,sp_runtime::generic::unchecked_extrinsic::UncheckedExtrinsic<sp_runtime::multiaddress::MultiAddress<<<sp_runtime::MultiSignature as sp_runtime::traits::Verify>::Signer as sp_runtime::traits::IdentifyAccount>::AccountId,()>,frequency_runtime::RuntimeCall,sp_runtime::MultiSignature,(frame_system::extensions::check_non_zero_sender::CheckNonZeroSender<frequency_runtime::Runtime>,frame_system::extensions::check_spec_version::CheckSpecVersion<frequency_runtime::Runtime>,frame_system::extensions::check_tx_version::CheckTxVersion<frequency_runtime::Runtime>,frame_system::extensions::check_genesis::CheckGenesis<frequency_runtime::Runtime>,frame_system::extensions::check_mortality::CheckMortality<frequency_runtime::Runtime>,common_runtime::extensions::check_nonce::CheckNonce<frequency_runtime::Runtime>,frame_system::extensions::check_weight::CheckWeight<frequency_runtime::Runtime>,pallet_transaction_payment::ChargeTransactionPayment<frequency_runtime::Runtime>,pallet_msa::CheckFreeExtrinsicUse<frequency_runtime::Runtime>)>>>>::dispatch_benchmark::ha1e526a41c28b52f\n    5: 0x1b10cc - <unknown>!Benchmark_dispatch_benchmark\n")
❌ 💔

@rlaferla
Copy link
Contributor

rlaferla commented Jan 26, 2023

The problem stems from the default values of 0 for CollatorMinCandidates and CollatorMaxCandidates for each of the three environments (local, testnet, mainnet.) It is unclear what these values should be. The benchmark panics when they are set to 0 when it should have an assert to test for invalid configurations. I may file an issue with Parity for it.

https://paritytech.github.io/cumulus/pallet_collator_selection/index.html

@rlaferla
Copy link
Contributor

After consulting with @wilwade , it was decided to make CollatorMinCandidates = 1 and CollatorMaxCandidates = 50

rlaferla added a commit that referenced this issue Jan 30, 2023
# Goal
The goal of this PR is to add benchmarking for
pallet-collator-selection.

Closes #608

---------

Co-authored-by: Jenkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore No feature changes tech debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants