Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

para scheduler: fix division #7339

Merged
merged 2 commits into from
Jun 11, 2023
Merged

para scheduler: fix division #7339

merged 2 commits into from
Jun 11, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Jun 7, 2023 •
edited
Loading

RIP Westend

There are a lot more cases where this number is used in arithmetic, so I assume that it should not be zero to beginn with.
So maybe one of the migrations caused it.

@ggwpez
para scheduler: fix division
309bc6b 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Jun 7, 2023 •
edited
Loading

I am actually going to close this since this cant be the root cause. The group rotating frequency is set to 10 in the previous block and should never be zero by invariant.
So something else must be wrong. If it was a faulty migration, it should error in the next block, but it failed in the same as the set_bode 🤔

@ggwpez ggwpez closed this Jun 7, 2023
@shawntabrizi
Copy link
Member

Still seems like a pretty safe thing to add?

@ggwpez
Copy link
Member Author

ggwpez commented Jun 7, 2023 •
edited
Loading

Yea right, it should not do any harm. The actual fix is in #7340

@ggwpez ggwpez reopened this Jun 7, 2023
@liamaharon liamaharon added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. T1-runtime This PR/Issue is related to the topic “runtime”. labels Jun 7, 2023
liamaharon
liamaharon approved these changes Jun 7, 2023
View reviewed changes
runtime/parachains/src/scheduler.rs Outdated Show resolved Hide resolved
@liamaharon liamaharon added A0-please_review Pull request needs code review. and removed A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). labels Jun 7, 2023
@kianenigma @liamaharon
Update runtime/parachains/src/scheduler.rs
f10c89d 
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
@gilescope
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit b6576d1 into master Jun 11, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-fix-panic branch June 11, 2023 06:28
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
@JesseAbram JesseAbram mentioned this pull request Oct 26, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@liamaharon liamaharon liamaharon approved these changes

@kianenigma kianenigma kianenigma approved these changes

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@gilescope gilescope gilescope approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ggwpez @shawntabrizi @gilescope @kianenigma @liamaharon