diff --git a/prdoc/pr_7790.prdoc b/prdoc/pr_7790.prdoc new file mode 100644 index 0000000000000..09a8ff52c8d13 --- /dev/null +++ b/prdoc/pr_7790.prdoc @@ -0,0 +1,9 @@ +title: 'pallet-scheduler: Put back postponed tasks into the agenda' +doc: +- audience: Runtime Dev + description: "Right now `pallet-scheduler` is not putting back postponed tasks into\ + \ the agenda when the early weight check is failing. This pull request ensures\ + \ that these tasks are put back into the agenda and are not just \"lost\".\r\n" +crates: +- name: pallet-scheduler + bump: patch diff --git a/substrate/frame/scheduler/src/lib.rs b/substrate/frame/scheduler/src/lib.rs index 2ad94ec04df47..65099dfd690cf 100644 --- a/substrate/frame/scheduler/src/lib.rs +++ b/substrate/frame/scheduler/src/lib.rs @@ -323,6 +323,7 @@ pub mod pallet { type BlockNumberProvider: BlockNumberProvider; } + /// Block number at which the agenda began incomplete execution. #[pallet::storage] pub type IncompleteSince = StorageValue<_, BlockNumberFor>; @@ -386,6 +387,8 @@ pub mod pallet { RetryFailed { task: TaskAddress>, id: Option }, /// The given task can never be executed since it is overweight. PermanentlyOverweight { task: TaskAddress>, id: Option }, + /// Agenda is incomplete from `when`. + AgendaIncomplete { when: BlockNumberFor }, } #[pallet::error] @@ -1202,6 +1205,7 @@ impl Pallet { } incomplete_since = incomplete_since.min(when); if incomplete_since <= now { + Self::deposit_event(Event::AgendaIncomplete { when: incomplete_since }); IncompleteSince::::put(incomplete_since); } } @@ -1235,10 +1239,7 @@ impl Pallet { let mut dropped = 0; for (agenda_index, _) in ordered.into_iter().take(max as usize) { - let task = match agenda[agenda_index as usize].take() { - None => continue, - Some(t) => t, - }; + let Some(task) = agenda[agenda_index as usize].take() else { continue }; let base_weight = T::WeightInfo::service_task( task.call.lookup_len().map(|x| x as usize), task.maybe_id.is_some(), @@ -1246,6 +1247,7 @@ impl Pallet { ); if !weight.can_consume(base_weight) { postponed += 1; + agenda[agenda_index as usize] = Some(task); break } let result = Self::service_task(weight, now, when, agenda_index, *executed == 0, task); diff --git a/substrate/frame/scheduler/src/mock.rs b/substrate/frame/scheduler/src/mock.rs index a9aea97542acd..28b5ee6d6083c 100644 --- a/substrate/frame/scheduler/src/mock.rs +++ b/substrate/frame/scheduler/src/mock.rs @@ -212,7 +212,7 @@ impl WeightInfo for TestWeightInfo { } } parameter_types! { - pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * + pub storage MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; } diff --git a/substrate/frame/scheduler/src/tests.rs b/substrate/frame/scheduler/src/tests.rs index d0a3acc05ac7e..1b7739e855ea7 100644 --- a/substrate/frame/scheduler/src/tests.rs +++ b/substrate/frame/scheduler/src/tests.rs @@ -1264,8 +1264,8 @@ fn cancel_named_periodic_scheduling_works() { #[test] fn scheduler_respects_weight_limits() { - let max_weight: Weight = ::MaximumWeight::get(); new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); let call = RuntimeCall::Logger(LoggerCall::log { i: 42, weight: max_weight / 3 * 2 }); assert_ok!(Scheduler::do_schedule( DispatchTime::At(4), @@ -1292,8 +1292,8 @@ fn scheduler_respects_weight_limits() { #[test] fn retry_respects_weight_limits() { - let max_weight: Weight = ::MaximumWeight::get(); new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); // schedule 42 let call = RuntimeCall::Logger(LoggerCall::log { i: 42, weight: max_weight / 3 * 2 }); assert_ok!(Scheduler::do_schedule( @@ -1344,8 +1344,8 @@ fn retry_respects_weight_limits() { #[test] fn try_schedule_retry_respects_weight_limits() { - let max_weight: Weight = ::MaximumWeight::get(); new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); let service_agendas_weight = ::WeightInfo::service_agendas_base(); let service_agenda_weight = ::WeightInfo::service_agenda_base( ::MaxScheduledPerBlock::get(), @@ -1404,8 +1404,8 @@ fn try_schedule_retry_respects_weight_limits() { /// Permanently overweight calls are not deleted but also not executed. #[test] fn scheduler_does_not_delete_permanently_overweight_call() { - let max_weight: Weight = ::MaximumWeight::get(); new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); let call = RuntimeCall::Logger(LoggerCall::log { i: 42, weight: max_weight }); assert_ok!(Scheduler::do_schedule( DispatchTime::At(4), @@ -1430,10 +1430,10 @@ fn scheduler_does_not_delete_permanently_overweight_call() { #[test] fn scheduler_handles_periodic_failure() { - let max_weight: Weight = ::MaximumWeight::get(); - let max_per_block = ::MaxScheduledPerBlock::get(); - new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); + let max_per_block = ::MaxScheduledPerBlock::get(); + let call = RuntimeCall::Logger(LoggerCall::log { i: 42, weight: (max_weight / 3) * 2 }); let bound = Preimage::bound(call).unwrap(); @@ -1472,9 +1472,9 @@ fn scheduler_handles_periodic_failure() { #[test] fn scheduler_handles_periodic_unavailable_preimage() { - let max_weight: Weight = ::MaximumWeight::get(); - new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); + let call = RuntimeCall::Logger(LoggerCall::log { i: 42, weight: (max_weight / 3) * 2 }); let hash = ::Hashing::hash_of(&call); let len = call.using_encoded(|x| x.len()) as u32; @@ -1518,8 +1518,8 @@ fn scheduler_handles_periodic_unavailable_preimage() { #[test] fn scheduler_respects_priority_ordering() { - let max_weight: Weight = ::MaximumWeight::get(); new_test_ext().execute_with(|| { + let max_weight: Weight = ::MaximumWeight::get(); let call = RuntimeCall::Logger(LoggerCall::log { i: 42, weight: max_weight / 3 }); assert_ok!(Scheduler::do_schedule( DispatchTime::At(4), @@ -3039,3 +3039,40 @@ fn unavailable_call_is_detected() { assert!(!Preimage::is_requested(&hash)); }); } + +#[test] +fn postponed_task_is_still_available() { + new_test_ext().execute_with(|| { + let service_agendas_weight = ::WeightInfo::service_agendas_base(); + let service_agenda_weight = ::WeightInfo::service_agenda_base( + ::MaxScheduledPerBlock::get(), + ); + + assert_ok!(Scheduler::schedule( + RuntimeOrigin::root(), + 4, + None, + 128, + Box::new(RuntimeCall::from(frame_system::Call::remark { + remark: vec![0u8; 3 * 1024 * 1024], + })) + )); + System::run_to_block::(3); + // Scheduled calls are in the agenda. + assert_eq!(Agenda::::get(4).len(), 1); + + let old_weight = MaximumSchedulerWeight::get(); + MaximumSchedulerWeight::set(&service_agenda_weight.saturating_add(service_agendas_weight)); + + System::run_to_block::(4); + + // The task should still be there. + assert_eq!(Agenda::::get(4).iter().filter(|a| a.is_some()).count(), 1); + System::assert_last_event(crate::Event::AgendaIncomplete { when: 4 }.into()); + + // Now it should get executed + MaximumSchedulerWeight::set(&old_weight); + System::run_to_block::(5); + assert!(Agenda::::get(4).is_empty()); + }); +}