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

Runtime: allow backing multiple candidates of same parachain on different cores #3231

Merged
merged 63 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
cab6ab9
Switch statement table from ParaId to CoreIndex
sandreim Feb 1, 2024
d2df658
cargo lock
sandreim Feb 2, 2024
4a8b8d5
add experimental feature
sandreim Feb 6, 2024
9244632
inject core_index from statements
sandreim Feb 6, 2024
22e017b
temporary provisioner fix
sandreim Feb 6, 2024
9dc8927
Support injected `CoreIndex`
sandreim Feb 6, 2024
574b06a
Merge branch 'master' of github.com:paritytech/polkadot-sdk into sand…
sandreim Feb 6, 2024
fbb7351
cargo lock
sandreim Feb 6, 2024
c6b833e
Merge branch 'sandreim/backing_multiple_cores_per_para' of github.com…
sandreim Feb 6, 2024
6c72918
It was damn hard to fix these tests
sandreim Feb 12, 2024
fc5c109
These tests were easy to fix
sandreim Feb 12, 2024
33351b4
Fix comment
sandreim Feb 12, 2024
0d994bf
clippy was angry
sandreim Feb 12, 2024
534c019
A bit refactor and add a test
sandreim Feb 12, 2024
10d86dd
taplo happy
sandreim Feb 12, 2024
d990a65
Merge branch 'sandreim/backing_multiple_cores_per_para' of github.com…
sandreim Feb 13, 2024
fbe1ad5
BackedCandidate: make all members private and provide an interface
sandreim Feb 13, 2024
e74f038
refactor based on new BackedCandidate
sandreim Feb 13, 2024
d898740
Fix all parachain runtime tests affected
sandreim Feb 13, 2024
9e40490
fix more broken test on node side
sandreim Feb 13, 2024
42f46a0
wip new test
sandreim Feb 14, 2024
222609c
review feedback
sandreim Feb 14, 2024
532d363
Merge branch 'sandreim/backing_multiple_cores_per_para' of github.com…
sandreim Feb 14, 2024
ccb2a88
ElasticScalingCoreIndex
sandreim Feb 14, 2024
a5ba157
finish filtering of candidates for elastic scaling
sandreim Feb 14, 2024
a02e896
remove log
sandreim Feb 14, 2024
dd34850
more feedback
sandreim Feb 14, 2024
838a846
Merge remote-tracking branch 'origin/master' into sandreim/backing_mu…
alindima Feb 19, 2024
ad98f18
use next up on available instead of occupied core index
alindima Feb 19, 2024
606d7c4
ElasticScalingCoreIndex -> ElasticScalingMVP
alindima Feb 19, 2024
6fb6b73
Merge remote-tracking branch 'origin/sandreim/backing_multiple_cores_…
alindima Feb 20, 2024
10f6486
rename ElasticScalingCoreIndex
alindima Feb 20, 2024
f9e178d
address some comments
alindima Feb 20, 2024
ec7b660
+1
alindima Feb 20, 2024
578850b
more comments
alindima Feb 20, 2024
362ff1e
add a backing test
alindima Feb 20, 2024
2385369
add rstest
alindima Feb 20, 2024
c793b89
small nits and typos
alindima Feb 20, 2024
8398bb1
Merge remote-tracking branch 'origin/master' into sandreim/backing_mu…
alindima Feb 20, 2024
27ec25b
Merge remote-tracking branch 'origin/sandreim/backing_multiple_cores_…
alindima Feb 20, 2024
9f70276
Merge remote-tracking branch 'origin/master' into sandreim/backing_mu…
alindima Feb 21, 2024
19c9a67
review comments
alindima Feb 21, 2024
d7b6ce8
add zombienet test
alindima Feb 21, 2024
afed2a8
fix existing unit tests
alindima Feb 21, 2024
7ea040d
add prdoc
alindima Feb 21, 2024
79f281b
fix clippy
alindima Feb 21, 2024
7521ed9
try fixing prdoc
alindima Feb 21, 2024
9c3dd5c
cache Validator->Group mapping
alindima Feb 21, 2024
0b0b6d1
Merge remote-tracking branch 'origin/sandreim/backing_multiple_cores_…
alindima Feb 21, 2024
7976e2f
lockfile
alindima Feb 21, 2024
4d6e797
add tests for backedcandidate functions
alindima Feb 21, 2024
4c36440
newlines
alindima Feb 22, 2024
af1cd82
use Arc to avoid cloning
alindima Feb 22, 2024
5cc5b8b
Merge branch 'sandreim/backing_multiple_cores_per_para' into sandreim…
alindima Feb 22, 2024
dc57adb
add check for parachain stall to zombienet test
alindima Feb 22, 2024
bb5968c
add more unit tests
alindima Feb 22, 2024
1ca7a70
Merge remote-tracking branch 'origin/master' into sandreim/runtime_co…
alindima Feb 22, 2024
eb345b0
fix clippy
alindima Feb 22, 2024
058c0c2
refactor
alindima Feb 22, 2024
d4c58bd
fix some bugs and add more unit tests
alindima Feb 23, 2024
cb41758
update some comments
alindima Feb 23, 2024
bffa4e9
review comments
alindima Feb 23, 2024
5d3a85d
fix unit test
alindima Feb 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
finish filtering of candidates for elastic scaling
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
  • Loading branch information
sandreim committed Feb 14, 2024
commit a5ba15722208e0aeeeff3ca0dd161bedc31a8b1d
2 changes: 1 addition & 1 deletion polkadot/runtime/parachains/src/inclusion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ impl<T: Config> Pallet<T> {

let core_index_enabled = configuration::Pallet::<T>::config()
Copy link
Member

Choose a reason for hiding this comment

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

Does this make sense? If we want to check for this feature at all in the runtime then we should only use it in validation: Reject any candidate that has superfluous bits if not set, but I don't really see value in that either. If provided, we might as well use it, instead of being stubborn.

Copy link
Contributor

Choose a reason for hiding this comment

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

I generally agree with you, but I think it's a chicken-and-egg situation. In order to know if we have the core index encoded in the validator_indices, we need to know the backing group size. In order to know the backing group, we need to know the core index

.node_features
.get(FeatureIndex::InjectCoreIndex as usize)
.get(FeatureIndex::ElasticScalingCoreIndex as usize)
.map(|b| *b)
.unwrap_or(false);
let minimum_backing_votes = configuration::Pallet::<T>::config().minimum_backing_votes;
Expand Down
4 changes: 2 additions & 2 deletions polkadot/runtime/parachains/src/paras_inherent/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::{
paras,
scheduler::{self, FreedReason},
shared::{self, AllowedRelayParentsTracker},
util::elastic_scaling_mvp_filter,
util::filter_elastic_scaling_candidates,
alindima marked this conversation as resolved.
Show resolved Hide resolved
ParaId,
};
use bitvec::prelude::BitVec;
Expand Down Expand Up @@ -592,7 +592,7 @@ impl<T: Config> Pallet<T> {

METRICS.on_candidates_processed_total(backed_candidates.len() as u64);

elastic_scaling_mvp_filter::<T>(&mut backed_candidates);
filter_elastic_scaling_candidates::<T>(&mut backed_candidates);
eskimor marked this conversation as resolved.
Show resolved Hide resolved

let SanitizedBackedCandidates { backed_candidates, votes_from_disabled_were_dropped } =
sanitize_backed_candidates::<T, _>(
Expand Down
102 changes: 70 additions & 32 deletions polkadot/runtime/parachains/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ use primitives::{
vstaging::node_features::FeatureIndex, BackedCandidate, CoreIndex, Id as ParaId,
PersistedValidationData, ValidatorIndex,
};
use sp_std::{collections::btree_set::BTreeSet, vec::Vec};
use sp_std::{
collections::{btree_map::BTreeMap, btree_set::BTreeSet},
vec::Vec,
};

use crate::{configuration, hrmp, paras, scheduler};
/// Make the persisted validation data for a particular parachain, a specified relay-parent and it's
Expand Down Expand Up @@ -101,14 +104,17 @@ pub fn take_active_subset<T: Clone>(active: &[ValidatorIndex], set: &[T]) -> Vec
subset
}

/// Filters out all candidates that have multiple cores assigned and no
/// On first pass it filters out all candidates that have multiple cores assigned and no
/// `CoreIndex` injected.
pub(crate) fn elastic_scaling_mvp_filter<T: configuration::Config + scheduler::Config>(
///
/// On second pass we filter out candidates with the same core index. This can happen if for example
/// collators distribute collations to multiple backing groups.
eskimor marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) fn filter_elastic_scaling_candidates<T: configuration::Config + scheduler::Config>(
candidates: &mut Vec<BackedCandidate<T::Hash>>,
) {
if !configuration::Pallet::<T>::config()
.node_features
.get(FeatureIndex::InjectCoreIndex as usize)
.get(FeatureIndex::ElasticScalingCoreIndex as usize)
.map(|b| *b)
.unwrap_or(false)
{
Expand All @@ -117,9 +123,38 @@ pub(crate) fn elastic_scaling_mvp_filter<T: configuration::Config + scheduler::C
return
}

// TODO: determine cores assigned to this para.
let multiple_cores_asigned = true;
candidates.retain(|candidate| !multiple_cores_asigned || has_core_index::<T>(candidate, true));
// A mapping from parachain to all assigned cores.
let mut cores_per_parachain: BTreeMap<ParaId, Vec<CoreIndex>> = BTreeMap::new();

for (core_index, para_id) in <scheduler::Pallet<T>>::scheduled_paras() {
cores_per_parachain.entry(para_id).or_default().push(core_index);
}

// We keep a candidate if the parachain has only one core assigned or if
alindima marked this conversation as resolved.
Show resolved Hide resolved
// a core index is provided by block author.
candidates.retain(|candidate| {
!cores_per_parachain
.get(&candidate.candidate().descriptor.para_id)
.map(|cores| cores.len())
.unwrap_or(0) >
1 || has_core_index::<T>(candidate, true)
});

let mut used_cores = BTreeSet::new();

// We keep one candidate per core in case multiple candidates of same para end up backed on same
// core. This can be further refined to pick the candidate that has the parenthead equal
// to the one in storage.
candidates.retain(|candidate| {
if let Some(core_index) = candidate.assumed_core_index(true) {
// Drop candidate if the core was already used by a previous candidate.
used_cores.insert(core_index)
} else {
// This shouldn't happen, but at this point the candidate without core index is fine
// since we know the para:core mapping is unique.
true
}
})
}

// Returns `true` if the candidate contains an injected `CoreIndex`.
Expand Down Expand Up @@ -157,13 +192,16 @@ fn has_core_index<T: configuration::Config + scheduler::Config>(
#[cfg(test)]
mod tests {

use bitvec::vec::BitVec;
use sp_std::vec::Vec;
use test_helpers::{dummy_candidate_descriptor, dummy_hash};
use bitvec::vec::BitVec;
use sp_std::vec::Vec;
use test_helpers::{dummy_candidate_descriptor, dummy_hash};

use crate::util::{has_core_index, split_active_subset, take_active_subset};
use primitives::{BackedCandidate, CandidateCommitments, CommittedCandidateReceipt, PersistedValidationData, ValidatorIndex};
use bitvec::bitvec;
use primitives::{
BackedCandidate, CandidateCommitments, CommittedCandidateReceipt, PersistedValidationData,
ValidatorIndex,
};

#[test]
fn take_active_subset_is_compatible_with_split_active_subset() {
Expand All @@ -176,28 +214,28 @@ use test_helpers::{dummy_candidate_descriptor, dummy_hash};
assert_eq!(selected, vec![1, 3, 7]);
}

pub fn dummy_bitvec(size: usize) -> BitVec<u8, bitvec::order::Lsb0> {
pub fn dummy_bitvec(size: usize) -> BitVec<u8, bitvec::order::Lsb0> {
bitvec![u8, bitvec::order::Lsb0; 0; size]
}

#[test]
fn has_core_index_works() {
let mut descriptor = dummy_candidate_descriptor(dummy_hash());
let empty_hash = sp_core::H256::zero();

descriptor.para_id = 1000.into();
descriptor.persisted_validation_data_hash = empty_hash;
let committed_receipt = CommittedCandidateReceipt {
descriptor,
commitments: CandidateCommitments::default(),
};

let candidate = BackedCandidate::new(
committed_receipt.clone(),
Vec::new(),
dummy_bitvec(5),
);

assert_eq!(has_core_index::<T: configuration::Config + scheduler::Config>(&candidate, false), false);
}
// #[test]
// fn has_core_index_works() {
// let mut descriptor = dummy_candidate_descriptor(dummy_hash());
// let empty_hash = sp_core::H256::zero();

// descriptor.para_id = 1000.into();
// descriptor.persisted_validation_data_hash = empty_hash;
// let committed_receipt = CommittedCandidateReceipt {
// descriptor,
// commitments: CandidateCommitments::default(),
// };

// let candidate = BackedCandidate::new(
// committed_receipt.clone(),
// Vec::new(),
// dummy_bitvec(5),
// );

// assert_eq!(has_core_index::<T: configuration::Config + scheduler::Config>(&candidate, false),
// false); }
}
Loading