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

Scheduler: Retry a Task on a Call Failure #3014

Closed
muharem opened this issue Jan 22, 2024 · 6 comments · Fixed by #3060
Closed

Scheduler: Retry a Task on a Call Failure #3014

muharem opened this issue Jan 22, 2024 · 6 comments · Fixed by #3060

Comments

@muharem
Copy link
Contributor

muharem commented Jan 22, 2024

Problem

The scheduler pallet, utilized for setting and execution delayed tasks/calls, currently abandons the task upon it's call failure. In specific situations (refer to the use case: PR, place in code), there is a requirement to implement tasks retries within a specified time period.

Possible Solution

Introduce optional retry setting for a task:

#[pallet::storage]
pub type Retries<T: Config> = StorageMap<
    _,
    _,
    TaskAddress<BlockNumberFor<T>>,
    // (total retries, retries left, period)
    (u8, u8, BlockNumberFor<T>),
>;

/// Users assess the possibility of overlapping between a periodic task and retries on their own.
pub fn set_retry(origin Origin,  when: BlockNumber, index: u32, retries: u8, period: BlockNumber);
pub fn set_retry_named(origin Origin, id: TaskName, retries: u8, period: BlockNumber);

fn service_agenda(...) {
  .....
  if call_failed {
     // check if any retries 
     // if any, schedule new task to retry and update/remove the retry record
  } else {
     // check if any retries 
     // if any, remove the retry record and create new if a task is periodic
  }
  .....
}
@muharem muharem changed the title Scheduler: Retry a Call on Failure Scheduler: Retry a Task on Call Failure Jan 22, 2024
@muharem muharem changed the title Scheduler: Retry a Task on Call Failure Scheduler: Retry a Task on a Call Failure Jan 22, 2024
@bkchr
Copy link
Member

bkchr commented Jan 24, 2024

Wouldn't it make more sense that you just schedule some call that can handle the failing payout and knows if it needs to be rescheduled or not? It could for example try to swap less or similar?

@ggwpez
Copy link
Member

ggwpez commented Jan 24, 2024

There is the Tasks API now, maybe you can use that instead of the scheduler? #1343

@muharem
Copy link
Contributor Author

muharem commented Feb 1, 2024

Wouldn't it make more sense that you just schedule some call that can handle the failing payout and knows if it needs to be rescheduled or not? It could for example try to swap less or similar?

@bkchr yes, that would at least provide a better api for a user. but more work to do. probably some extension pallet for treasury pallet, to check payout result and extension pallet for asset-conversion, to check and retry swaps.
the goal for that template call was to explore if we can construct such a call today and share it with the community.

for a quick win, I would try to solve it with the scheduler, if the proposed solution for retries looks good and has not concerns. then, the retries looks to me as a good feature for the scheduler.

in the existing swap api, we already can set max_amoint_in or min_amount_out, which let us to accept some slippage.

There is the Tasks API now, maybe you can use that instead of the scheduler? #1343

if I understand it right, to utilize tasks, we need some extension pallet for asset conversion. because we cannot set some general task with an extrinsic.

if you think we should go with the extension pallet solution, I can explore it more. and I will need @joepetrowski help, to propiritize it.

@ggwpez
Copy link
Member

ggwpez commented Feb 1, 2024

@georgepisaltu is working on adding retires to the scheduler here: #3060

@muharem
Copy link
Contributor Author

muharem commented Feb 2, 2024

@georgepisaltu is working on adding retires to the scheduler here: #3060

yes, I'seen, but still wanna have some alignment on the solution.

@gupnik
Copy link
Contributor

gupnik commented Feb 2, 2024

if I understand it right, to utilize tasks, we need some extension pallet for asset conversion. because we cannot set some general task with an extrinsic.

@muharem A task is essentially just an extrinsic that accepts a certain set of inputs based on a validation criterion. Any task added to the runtime can be triggered via do_task extrinsic in frame_system. You would probably need an off-chain worker though to trigger such a task when the condition is validated. I might need to understand the context here a bit better but happy to brainstorm if needed.

github-merge-queue bot pushed a commit that referenced this issue Feb 16, 2024
Fixes #3014 

This PR adds retry mechanics to `pallet-scheduler`, as described in the
issue above.

Users can now set a retry configuration for a task so that, in case its
scheduled run fails, it will be retried after a number of blocks, for a
specified number of times or until it succeeds.

If a retried task runs successfully before running out of retries, its
remaining retry counter will be reset to the initial value. If a retried
task runs out of retries, it will be removed from the schedule.

Tasks which need to be scheduled for a retry are still subject to weight
metering and agenda space, same as a regular task. Periodic tasks will
have their periodic schedule put on hold while the task is retrying.

---------

Signed-off-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
Fixes paritytech#3014 

This PR adds retry mechanics to `pallet-scheduler`, as described in the
issue above.

Users can now set a retry configuration for a task so that, in case its
scheduled run fails, it will be retried after a number of blocks, for a
specified number of times or until it succeeds.

If a retried task runs successfully before running out of retries, its
remaining retry counter will be reset to the initial value. If a retried
task runs out of retries, it will be removed from the schedule.

Tasks which need to be scheduled for a retry are still subject to weight
metering and agenda space, same as a regular task. Periodic tasks will
have their periodic schedule put on hold while the task is retrying.

---------

Signed-off-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants