Skip to content

Commit f9cdf41

Browse files
authored
[pallet-broker] add extrinsic to reserve a system core without having to wait two sale boundaries (#4273)
When calling the reserve extrinsic after sales have started, the assignment will be reserved, but two sale period boundaries must pass before the core is actually assigned. Since this can take between 28 and 56 days on production networks, a new extrinsic is introduced to shorten the timeline. This essentially performs three actions: 1. Reserve it (applies after two sale boundaries) 2. Add it to the Workplan for the next sale period 3. Add it to the Workplan for the rest of the current sale period from the next timeslice to be commmitted. The caller must ensure that a core is first added, with most relay chain implementations having a delay of two session boundaries until it comes into effect. Alternatively the extrinsic can be called on a core whose workload can be clobbered from now until the reservation kicks in (the sale period after the next). Any workplan entries for that core at other timeslices should be first removed by the caller. --------- Co-authored-by: command-bot <>
1 parent d0c8a07 commit f9cdf41

File tree

8 files changed

+481
-0
lines changed

8 files changed

+481
-0
lines changed

cumulus/parachains/runtimes/coretime/coretime-rococo/src/weights/pallet_broker.rs

+18
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,24 @@ impl<T: frame_system::Config> pallet_broker::WeightInfo for WeightInfo<T> {
555555
.saturating_add(T::DbWeight::get().reads(5))
556556
.saturating_add(T::DbWeight::get().writes(1))
557557
}
558+
/// Storage: `Broker::SaleInfo` (r:1 w:0)
559+
/// Proof: `Broker::SaleInfo` (`max_values`: Some(1), `max_size`: Some(57), added: 552, mode: `MaxEncodedLen`)
560+
/// Storage: `Broker::Reservations` (r:1 w:1)
561+
/// Proof: `Broker::Reservations` (`max_values`: Some(1), `max_size`: Some(12021), added: 12516, mode: `MaxEncodedLen`)
562+
/// Storage: `Broker::Status` (r:1 w:0)
563+
/// Proof: `Broker::Status` (`max_values`: Some(1), `max_size`: Some(18), added: 513, mode: `MaxEncodedLen`)
564+
/// Storage: `Broker::Workplan` (r:0 w:2)
565+
/// Proof: `Broker::Workplan` (`max_values`: None, `max_size`: Some(1216), added: 3691, mode: `MaxEncodedLen`)
566+
fn force_reserve() -> Weight {
567+
// Proof Size summary in bytes:
568+
// Measured: `11125`
569+
// Estimated: `13506`
570+
// Minimum execution time: 32_286_000 picoseconds.
571+
Weight::from_parts(33_830_000, 0)
572+
.saturating_add(Weight::from_parts(0, 13506))
573+
.saturating_add(T::DbWeight::get().reads(3))
574+
.saturating_add(T::DbWeight::get().writes(3))
575+
}
558576
/// Storage: `Broker::Leases` (r:1 w:1)
559577
/// Proof: `Broker::Leases` (`max_values`: Some(1), `max_size`: Some(401), added: 896, mode: `MaxEncodedLen`)
560578
fn swap_leases() -> Weight {

cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/pallet_broker.rs

+18
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,24 @@ impl<T: frame_system::Config> pallet_broker::WeightInfo for WeightInfo<T> {
553553
.saturating_add(T::DbWeight::get().reads(5))
554554
.saturating_add(T::DbWeight::get().writes(1))
555555
}
556+
/// Storage: `Broker::SaleInfo` (r:1 w:0)
557+
/// Proof: `Broker::SaleInfo` (`max_values`: Some(1), `max_size`: Some(57), added: 552, mode: `MaxEncodedLen`)
558+
/// Storage: `Broker::Reservations` (r:1 w:1)
559+
/// Proof: `Broker::Reservations` (`max_values`: Some(1), `max_size`: Some(12021), added: 12516, mode: `MaxEncodedLen`)
560+
/// Storage: `Broker::Status` (r:1 w:0)
561+
/// Proof: `Broker::Status` (`max_values`: Some(1), `max_size`: Some(18), added: 513, mode: `MaxEncodedLen`)
562+
/// Storage: `Broker::Workplan` (r:0 w:2)
563+
/// Proof: `Broker::Workplan` (`max_values`: None, `max_size`: Some(1216), added: 3691, mode: `MaxEncodedLen`)
564+
fn force_reserve() -> Weight {
565+
// Proof Size summary in bytes:
566+
// Measured: `11125`
567+
// Estimated: `13506`
568+
// Minimum execution time: 31_464_000 picoseconds.
569+
Weight::from_parts(32_798_000, 0)
570+
.saturating_add(Weight::from_parts(0, 13506))
571+
.saturating_add(T::DbWeight::get().reads(3))
572+
.saturating_add(T::DbWeight::get().writes(3))
573+
}
556574
/// Storage: `Broker::Leases` (r:1 w:1)
557575
/// Proof: `Broker::Leases` (`max_values`: Some(1), `max_size`: Some(81), added: 576, mode: `MaxEncodedLen`)
558576
fn swap_leases() -> Weight {

prdoc/pr_4273.prdoc

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
2+
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
3+
4+
title: "[pallet-broker] add extrinsic to reserve a system core without having to wait two sale boundaries"
5+
6+
doc:
7+
- audience: Runtime User
8+
description: |
9+
When calling the reserve extrinsic after sales have started, the assignment will be reserved,
10+
but two sale period boundaries must pass before the core is actually assigned. A new
11+
`force_reserve` extrinsic is introduced to allow a core to be immediately assigned.
12+
13+
crates:
14+
- name: pallet-broker
15+
bump: major
16+
- name: coretime-rococo-runtime
17+
bump: patch
18+
- name: coretime-westend-runtime
19+
bump: patch

substrate/frame/broker/src/benchmarking.rs

+41
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,47 @@ mod benches {
10161016
Ok(())
10171017
}
10181018

1019+
#[benchmark]
1020+
fn force_reserve() -> Result<(), BenchmarkError> {
1021+
Configuration::<T>::put(new_config_record::<T>());
1022+
// Assume Reservations to be almost filled for worst case.
1023+
let reservation_count = T::MaxReservedCores::get().saturating_sub(1);
1024+
setup_reservations::<T>(reservation_count);
1025+
1026+
// Assume leases to be filled for worst case
1027+
setup_leases::<T>(T::MaxLeasedCores::get(), 1, 10);
1028+
1029+
let origin =
1030+
T::AdminOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
1031+
1032+
// Sales must be started.
1033+
Broker::<T>::do_start_sales(100u32.into(), CoreIndex::try_from(reservation_count).unwrap())
1034+
.map_err(|_| BenchmarkError::Weightless)?;
1035+
1036+
// Add a core.
1037+
let status = Status::<T>::get().unwrap();
1038+
Broker::<T>::do_request_core_count(status.core_count + 1).unwrap();
1039+
1040+
advance_to::<T>(T::TimeslicePeriod::get().try_into().ok().unwrap());
1041+
let schedule = new_schedule();
1042+
1043+
#[extrinsic_call]
1044+
_(origin as T::RuntimeOrigin, schedule.clone(), status.core_count);
1045+
1046+
assert_eq!(Reservations::<T>::decode_len().unwrap(), T::MaxReservedCores::get() as usize);
1047+
1048+
let sale_info = SaleInfo::<T>::get().unwrap();
1049+
assert_eq!(
1050+
Workplan::<T>::get((sale_info.region_begin, status.core_count)),
1051+
Some(schedule.clone())
1052+
);
1053+
// We called at timeslice 1, therefore 2 was already processed and 3 is the next possible
1054+
// assignment point.
1055+
assert_eq!(Workplan::<T>::get((3, status.core_count)), Some(schedule));
1056+
1057+
Ok(())
1058+
}
1059+
10191060
#[benchmark]
10201061
fn swap_leases() -> Result<(), BenchmarkError> {
10211062
let admin_origin =

substrate/frame/broker/src/dispatchable_impls.rs

+21
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,27 @@ impl<T: Config> Pallet<T> {
6060
Ok(())
6161
}
6262

63+
pub(crate) fn do_force_reserve(workload: Schedule, core: CoreIndex) -> DispatchResult {
64+
// Sales must have started, otherwise reserve is equivalent.
65+
let sale = SaleInfo::<T>::get().ok_or(Error::<T>::NoSales)?;
66+
67+
// Reserve - starts at second sale period boundary from now.
68+
Self::do_reserve(workload.clone())?;
69+
70+
// Add to workload - grants one region from the next sale boundary.
71+
Workplan::<T>::insert((sale.region_begin, core), &workload);
72+
73+
// Assign now until the next sale boundary unless the next timeslice is already the sale
74+
// boundary.
75+
let status = Status::<T>::get().ok_or(Error::<T>::Uninitialized)?;
76+
let timeslice = status.last_committed_timeslice.saturating_add(1);
77+
if timeslice < sale.region_begin {
78+
Workplan::<T>::insert((timeslice, core), &workload);
79+
}
80+
81+
Ok(())
82+
}
83+
6384
pub(crate) fn do_set_lease(task: TaskId, until: Timeslice) -> DispatchResult {
6485
let mut r = Leases::<T>::get();
6586
ensure!(until > Self::current_timeslice(), Error::<T>::AlreadyExpired);

substrate/frame/broker/src/lib.rs

+26
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,9 @@ pub mod pallet {
585585

586586
/// Reserve a core for a workload.
587587
///
588+
/// The workload will be given a reservation, but two sale period boundaries must pass
589+
/// before the core is actually assigned.
590+
///
588591
/// - `origin`: Must be Root or pass `AdminOrigin`.
589592
/// - `workload`: The workload which should be permanently placed on a core.
590593
#[pallet::call_index(1)]
@@ -943,6 +946,29 @@ pub mod pallet {
943946
Ok(())
944947
}
945948

949+
/// Reserve a core for a workload immediately.
950+
///
951+
/// - `origin`: Must be Root or pass `AdminOrigin`.
952+
/// - `workload`: The workload which should be permanently placed on a core starting
953+
/// immediately.
954+
/// - `core`: The core to which the assignment should be made until the reservation takes
955+
/// effect. It is left to the caller to either add this new core or reassign any other
956+
/// tasks to this existing core.
957+
///
958+
/// This reserves the workload and then injects the workload into the Workplan for the next
959+
/// two sale periods. This overwrites any existing assignments for this core at the start of
960+
/// the next sale period.
961+
#[pallet::call_index(23)]
962+
pub fn force_reserve(
963+
origin: OriginFor<T>,
964+
workload: Schedule,
965+
core: CoreIndex,
966+
) -> DispatchResultWithPostInfo {
967+
T::AdminOrigin::ensure_origin_or_root(origin)?;
968+
Self::do_force_reserve(workload, core)?;
969+
Ok(Pays::No.into())
970+
}
971+
946972
#[pallet::call_index(99)]
947973
#[pallet::weight(T::WeightInfo::swap_leases())]
948974
pub fn swap_leases(origin: OriginFor<T>, id: TaskId, other: TaskId) -> DispatchResult {

0 commit comments

Comments
 (0)