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

refactor(node)!: Store only first missing block in scheduler #2404

Merged
merged 42 commits into from
May 15, 2023

Conversation

ark0f
Copy link
Member

@ark0f ark0f commented Mar 15, 2023

Resolves #1333

A little internal optimization. We stored a set of block numbers whose on-chain tasks were skipped so we can process them later. Now we store only first block and process tasks up to current block (including). It makes internal algorithm a little bit simpler and uses less storage memory.

@ark0f ark0f added A0-pleasereview PR is ready to be reviewed by the team D2-node Gear Node labels Mar 15, 2023
@ark0f ark0f requested a review from breathx March 15, 2023 16:13
Copy link
Member

@breathx breathx left a comment

Choose a reason for hiding this comment

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

Migrations are must have here, so I'd switch atm on #2303 and merge it once mentioned one got solved

common/src/scheduler/mod.rs Outdated Show resolved Hide resolved
common/src/scheduler/mod.rs Outdated Show resolved Hide resolved
common/src/scheduler/mod.rs Outdated Show resolved Hide resolved
pallets/gear-scheduler/src/lib.rs Outdated Show resolved Hide resolved
@breathx breathx added A3-gotissues PR occurred to have issues after the review and removed A0-pleasereview PR is ready to be reviewed by the team labels Mar 26, 2023
@ark0f ark0f requested a review from breathx March 27, 2023 18:09
@ark0f ark0f added A0-pleasereview PR is ready to be reviewed by the team and removed A3-gotissues PR occurred to have issues after the review labels Apr 18, 2023
@ark0f ark0f changed the title Store only first missing block in scheduler refactor(node)!: Store only first missing block in scheduler Apr 24, 2023
@shamilsan shamilsan changed the title refactor(node)!: Store only first missing block in scheduler refactor(node): Store only first missing block in scheduler Apr 25, 2023
@shamilsan shamilsan changed the title refactor(node): Store only first missing block in scheduler refactor(node)!: Store only first missing block in scheduler Apr 25, 2023
@ark0f ark0f mentioned this pull request Apr 25, 2023
Copy link
Member

@breathx breathx left a comment

Choose a reason for hiding this comment

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

case:
block N contains no tasks, first missed block = None
block N+1 contains tasks, but block producer missed run_queue extrinsic or runtime upgrade occurs
block N+2 contains tasks and starts execute them because missed blocks = None so tasks from block N+1 lost forever

@ark0f ark0f requested a review from breathx April 26, 2023 11:07
@shamilsan
Copy link
Contributor

@ark0f Are we ready to merge this brilliant PR?

@breathx
Copy link
Member

breathx commented May 15, 2023

@ark0f merge master, please, and lets merge it

@shamilsan shamilsan added A2-mergeoncegreen PR is ready to merge after CI passes and removed A0-pleasereview PR is ready to be reviewed by the team labels May 15, 2023
# Conflicts:
#	runtime/gear/src/migrations.rs
#	runtime/vara/src/migrations.rs
@ark0f ark0f merged commit ffd3243 into master May 15, 2023
8 checks passed
@ark0f ark0f deleted the al-min-missed-block branch May 15, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A2-mergeoncegreen PR is ready to merge after CI passes D2-node Gear Node
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimise Scheduler's MissedBlocks by replacing with minimal value
4 participants