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

Add retry mechanics to pallet-scheduler #3060

Merged
merged 44 commits into from
Feb 16, 2024
Merged
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
328d0cd
Add option to retry task in scheduler
georgepisaltu Jan 23, 2024
6f56d9f
Add unit tests for retry scheduler
georgepisaltu Jan 23, 2024
22800c5
Add benchmarks for retry scheduler
georgepisaltu Jan 24, 2024
9315c2e
Add some docs to retry functions
georgepisaltu Jan 25, 2024
5bbd31e
Remove redundant clone
georgepisaltu Jan 25, 2024
3eb8016
Add real weights to scheduler pallet
georgepisaltu Jan 25, 2024
9ecae46
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Jan 25, 2024
fcff15c
".git/.scripts/commands/bench-all/bench-all.sh" --pallet=pallet_sched…
Jan 25, 2024
a8fd732
".git/.scripts/commands/bench-all/bench-all.sh" --pallet=pallet_sched…
Jan 25, 2024
3e6e77e
Merge branch 'master' of https://github.com/paritytech/polkadot-sdk i…
Jan 25, 2024
9120d60
".git/.scripts/commands/bench-all/bench-all.sh" --pallet=pallet_sched…
Jan 25, 2024
4167d6f
".git/.scripts/commands/bench-all/bench-all.sh" --pallet=pallet_sched…
Jan 25, 2024
816906a
Use `TaskAddress` in `set_retry`
georgepisaltu Jan 26, 2024
8a28ea5
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Jan 26, 2024
900d2d5
Add prdoc
georgepisaltu Jan 26, 2024
05cbdfc
Refactor agenda query in `set_retry`
georgepisaltu Jan 26, 2024
5fc62f4
Minor renames and fixes
georgepisaltu Jan 26, 2024
5e305e1
Refactor `schedule_retry` return type
georgepisaltu Jan 26, 2024
5943dcc
Implement `ensure_privilege`
georgepisaltu Jan 26, 2024
690f2b8
Add event for setting retry config
georgepisaltu Jan 26, 2024
4a81417
Make retry fail if insufficient weight
georgepisaltu Jan 29, 2024
438effb
Remove redundant weight parameter in `set_retry`
georgepisaltu Jan 29, 2024
d048760
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Jan 29, 2024
3c2b540
Add test for dropping insufficient weight retry
georgepisaltu Jan 29, 2024
7a39a69
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Jan 29, 2024
2e8f954
Clean up retry config on cancel
georgepisaltu Feb 1, 2024
7dfb517
Small refactor
georgepisaltu Feb 1, 2024
2e26707
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 1, 2024
40b567d
Add docs to retry config map
georgepisaltu Feb 1, 2024
2b35465
Add retry count to `RetrySet` event
georgepisaltu Feb 2, 2024
a43df52
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 2, 2024
9da58c6
Make retries independent of periodic runs
georgepisaltu Feb 7, 2024
2976dac
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 7, 2024
8ce66f7
Small refactoring
georgepisaltu Feb 13, 2024
dc9ef2e
Add `cancel_retry` extrinsics
georgepisaltu Feb 13, 2024
945a095
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 13, 2024
39eb209
Add e2e unit test for retry schedule
georgepisaltu Feb 14, 2024
2290240
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 14, 2024
50a2010
Simplify `schedule_retry`
georgepisaltu Feb 15, 2024
e9cc27e
Add docs for `as_retry`
georgepisaltu Feb 15, 2024
497100c
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 15, 2024
863bec7
Update doc comments for `set_retry`
georgepisaltu Feb 16, 2024
7a16648
Merge remote-tracking branch 'upstream/master' into retry-schedule
georgepisaltu Feb 16, 2024
b72da0c
Move common logic under `do_cancel_retry`
georgepisaltu Feb 16, 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
Add event for setting retry config
Signed-off-by: georgepisaltu <[email protected]>
  • Loading branch information
georgepisaltu committed Jan 26, 2024
commit 690f2b8a4cb7d83366f4d824c704acdf1b590907
26 changes: 21 additions & 5 deletions substrate/frame/scheduler/src/benchmarking.rs
Original file line number Diff line number Diff line change
@@ -27,14 +27,22 @@ use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use sp_std::{prelude::*, vec};

use crate::Pallet as Scheduler;
use frame_system::Call as SystemCall;
use frame_system::{Call as SystemCall, EventRecord};

const SEED: u32 = 0;

const BLOCK_NUMBER: u32 = 2;

type SystemOrigin<T> = <T as frame_system::Config>::RuntimeOrigin;

fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
let events = frame_system::Pallet::<T>::events();
let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();
// compare to the last event record
let EventRecord { event, .. } = &events[events.len() - 1];
assert_eq!(event, &system_event);
}

/// Add `n` items to the schedule.
///
/// For `resolved`:
@@ -336,11 +344,15 @@ benchmarks! {
let name = u32_to_name(s - 1);
let address = Lookup::<T>::get(name).unwrap();
let (when, index) = address;
}: _(RawOrigin::Root, (when, index), 10, BlockNumberFor::<T>::one())
let period = BlockNumberFor::<T>::one();
}: _(RawOrigin::Root, (when, index), 10, period)
verify {
assert_eq!(
Retries::<T>::get((when, index)),
Some(RetryConfig { total_retries: 10, remaining: 10, period: BlockNumberFor::<T>::one() })
Some(RetryConfig { total_retries: 10, remaining: 10, period })
);
assert_last_event::<T>(
Event::RetrySet { task: address, id: None, period }.into(),
);
}

@@ -352,11 +364,15 @@ benchmarks! {
let name = u32_to_name(s - 1);
let address = Lookup::<T>::get(name).unwrap();
let (when, index) = address;
}: _(RawOrigin::Root, name, 10, BlockNumberFor::<T>::one())
let period = BlockNumberFor::<T>::one();
}: _(RawOrigin::Root, name, 10, period)
verify {
assert_eq!(
Retries::<T>::get((when, index)),
Some(RetryConfig { total_retries: 10, remaining: 10, period: BlockNumberFor::<T>::one() })
Some(RetryConfig { total_retries: 10, remaining: 10, period })
);
assert_last_event::<T>(
Event::RetrySet { task: address, id: Some(name), period }.into(),
);
}

20 changes: 16 additions & 4 deletions substrate/frame/scheduler/src/lib.rs
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -315,6 +315,12 @@ pub mod pallet {
id: Option<TaskName>,
result: DispatchResult,
},
/// Set a retry configuration for some task.
RetrySet {
task: TaskAddress<BlockNumberFor<T>>,
id: Option<TaskName>,
period: BlockNumberFor<T>,
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
},
/// The call for the provided hash was not found so the task has been aborted.
CallUnavailable { task: TaskAddress<BlockNumberFor<T>>, id: Option<TaskName> },
/// The given task was unable to be renewed since the agenda is full at that block.
@@ -486,15 +492,16 @@ pub mod pallet {
let origin = <T as Config>::RuntimeOrigin::from(origin);
let (when, index) = task;
let agenda = Agenda::<T>::get(when);
let task = agenda
let scheduled = agenda
.get(index as usize)
.and_then(Option::as_ref)
.ok_or(Error::<T>::NotFound)?;
Self::ensure_privilege(origin.caller(), &task.origin)?;
Self::ensure_privilege(origin.caller(), &scheduled.origin)?;
Retries::<T>::insert(
(when, index),
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
RetryConfig { total_retries: retries, remaining: retries, period },
);
Self::deposit_event(Event::RetrySet { task, id: None, period });
Ok(())
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
}

@@ -521,15 +528,20 @@ pub mod pallet {
let origin = <T as Config>::RuntimeOrigin::from(origin);
let (when, agenda_index) = Lookup::<T>::get(&id).ok_or(Error::<T>::NotFound)?;
let agenda = Agenda::<T>::get(when);
let task = agenda
let scheduled = agenda
Copy link
Member

Choose a reason for hiding this comment

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

Looking at our internal code this should probably be refactored into a do_set_retry(name: Either<TaskName, TaskAddress>) but i dont want to block this any further.

.get(agenda_index as usize)
.and_then(Option::as_ref)
.ok_or(Error::<T>::NotFound)?;
Self::ensure_privilege(origin.caller(), &task.origin)?;
Self::ensure_privilege(origin.caller(), &scheduled.origin)?;
Retries::<T>::insert(
(when, agenda_index),
RetryConfig { total_retries: retries, remaining: retries, period },
);
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
Self::deposit_event(Event::RetrySet {
task: (when, agenda_index),
id: Some(id),
period,
});
Ok(())
}
}
Loading