Skip to content

Scheduler on_initialize supports skipped blocks#8723

Merged
muharem merged 6 commits intomasterfrom
muharem-scheduler-support-missed-blocks
Jun 21, 2025
Merged

Scheduler on_initialize supports skipped blocks#8723
muharem merged 6 commits intomasterfrom
muharem-scheduler-support-missed-blocks

Conversation

@muharem
Copy link
Copy Markdown
Contributor

@muharem muharem commented Jun 2, 2025

Scheduler on_initialize supports skipped blocks.

Scheduler correctly handles situations where on_initialize is invoked with block numbers that:

  • increase but are not strictly consecutive (e.g., jump from 5 → 10), or
  • are repeated (e.g., multiple blocks are built at the same Relay Chain parent block, all reporting the same BlockNumberProvider value).

This situation may occur when the BlockNumberProvider is not local - for example, on a parachain using the Relay Chain block number provider.

Implementation notes:

  • The IncompleteSince value is always set to the next block (now + 1).
  • A scheduled task is considered permanently overweight only if it fails during the first agenda processing;

@muharem muharem requested a review from a team as a code owner June 2, 2025 12:28
@muharem muharem added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Jun 2, 2025
@muharem muharem requested a review from ggwpez June 2, 2025 14:03
// However, if the [`Config::BlockNumberProvider`] is not a local block number provider,
// then `next_iter_now` could be `now + n` where `n > 1`. In this case, we want to start
// from `now + 1` to ensure we don't miss any agendas.
IncompleteSince::<T>::put(now + One::one());
Copy link
Copy Markdown
Member

@ggwpez ggwpez Jun 2, 2025

Choose a reason for hiding this comment

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

So we basically "abuse" IncompleteSince to just be a cursor that always tracks the next number, yea should work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also considered adding a new storage item like NextBlock, in this case (1) it becomes exactly what is IncompleteSince with this PR changes or (2) it always shows next block from last iteration and we need to manage both storage values, making code a bit more complex.

@muharem muharem requested a review from bkchr June 3, 2025 15:40
@muharem muharem requested review from acatangiu, bkontur and gui1117 June 10, 2025 11:18
Copy link
Copy Markdown
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

This PR is good to me.

How can we ensure a similar mistake wasn't made in other pallets when migrating to the different block provider?
Should we re-check them and specifically check that they didn't use the property that block numbers are continuous?

Comment thread substrate/frame/scheduler/src/lib.rs
@ggwpez
Copy link
Copy Markdown
Member

ggwpez commented Jun 17, 2025

How can we ensure a similar mistake wasn't made in other pallets when migrating to the different block provider?

We documented the block number providers in the pallet that they do not support hot-swapping and that it is best to stay with System to avoid trouble. So currently the avoidance is "read the docs" 😆 (but we know nobody does that)

@gui1117
Copy link
Copy Markdown
Contributor

gui1117 commented Jun 18, 2025

How can we ensure a similar mistake wasn't made in other pallets when migrating to the different block provider?

We documented the block number providers in the pallet that they do not support hot-swapping and that it is best to stay with System to avoid trouble. So currently the avoidance is "read the docs" 😆 (but we know nobody does that)

I mean for this pallet we didn't document that the block provider must be increasing only by one between blocks.

It said:

  /// Must return monotonically increasing values when called from consecutive blocks. It is
  /// generally expected that the values also do not differ "too much" between consecutive
  /// blocks. A future addition to this pallet will allow bigger difference between
  /// consecutive blocks to make it possible to be utilized by parachains with *Agile
  /// Coretime*.

Which was wrong, the pallet didn't support any difference more than 1 between consecutive blocks, but the doc said it supported any difference that is not "too much".

@muharem
Copy link
Copy Markdown
Contributor Author

muharem commented Jun 18, 2025

@gui1117 @ggwpez I also added a TODO for myself to check other pallets that were recently updated to use the block number provider.

@muharem muharem added this pull request to the merge queue Jun 21, 2025
Merged via the queue into master with commit 5072bf9 Jun 21, 2025
246 checks passed
@muharem muharem deleted the muharem-scheduler-support-missed-blocks branch June 21, 2025 21:15
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
Scheduler `on_initialize` supports skipped blocks.

Scheduler correctly handles situations where `on_initialize` is invoked
with block numbers that:
- increase but are not strictly consecutive (e.g., jump from 5 → 10), or
- are repeated (e.g., multiple blocks are built at the same Relay Chain
parent block, all reporting the same `BlockNumberProvider` value).

This situation may occur when the `BlockNumberProvider` is not local -
for example, on a parachain using the Relay Chain block number provider.

Implementation notes:
- The `IncompleteSince` value is always set to the next block `(now +
1)`.
- A scheduled task is considered permanently overweight only if it fails
during the first agenda processing;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T1-FRAME This PR/Issue is related to core FRAME, the framework.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants