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.

Sign up for GitHub

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

Jump to bottom

[MQ pallet] Dont warn for low on_idle weight #4762

Closed
ggwpez opened this issue Jun 11, 2024 · 4 comments
Closed

[MQ pallet] Dont warn for low on_idle weight #4762

ggwpez opened this issue Jun 11, 2024 · 4 comments
Assignees
@gotnoshoeson
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.

Comments

@ggwpez
Copy link
Member

ggwpez commented Jun 11, 2024 •
edited
Loading

The on_idle of the MQ pallet calls the service_queues function which prints a warning when it does not have enough weight to do anything.
Normally this is an issue since its called from on_initialize, but on_idle it should not print a defensive error.
It was introduced in #3844.
@ggwpez ggwpez added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Jun 11, 2024
@ggwpez ggwpez changed the title [MQ pallet] Remove warn for low on_idle weight [MQ pallet] Dont warn for low on_idle weight Jun 11, 2024
@gotnoshoeson
Copy link
Contributor

Yo @ggwpez looking for a good-first-issue to jump into polkadot and contribute. Coming from Ethereum full-stack app development.

@gotnoshoeson
Copy link
Contributor

gotnoshoeson commented Jun 12, 2024 •
edited
Loading

So I see the defensive! getting thrown on line 1562. defensive!("Not enough weight to service a single message."); I'm thinking I can create a context parameter of where the service_queues is being called. fn service_queues(weight_limit: Weight, context: Context) If it's being called within on_initialize then pass "on_initialize" as the argument and it can continue with the current code. If "on_idle" is passed as context argument then it just returns Weight::zero(). Does this logic make sense? Or is there some other convention for doing something like this in Rust? Cheers!

@ggwpez
Copy link
Member Author

ggwpez commented Jun 12, 2024

Yo ggwpez looking for a good-first-issue to jump into polkadot and contribute. Coming from Ethereum full-stack app development.

Thanks, yes please go ahead.

fn service_queues(weight_limit: Weight, context: Context)

Yea that sounds good! I think we dont have a precedent case on how to handle this, but a small enum with two entries OnIdle and OnInitialize should work 😄

@gotnoshoeson gotnoshoeson mentioned this issue Jun 15, 2024
Merged
@vgantchev vgantchev mentioned this issue Oct 16, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 16, 2024
@gotnoshoeson @bkchr
Fix for Issue 4762 (#4803)
504edb1 
[Issue #4762 ](#4762)

- Creates an enum for passing context of `service_queues` being called
from within `on_initialize` and `on_idle` hooks. Uses a match statement
inside of `service_queues` to continue using the same code, but NOT
throw a `defensive` if being called within `on_idle` hook.
- The issue requested to not throw the `defensive` if being called from
within `on_idle` hook.
- Created the `ServiceQueuesContext` enum to pass as an argument of
`service_queues` when called within the `on_initialize` and `on_idle`
hooks. A match statement was added inside of `service_queues` to
continue to throw the defensive if called from `on_initialize` but NOT
throw the defensive if called from `on_idle`.

---------

Co-authored-by: gotnoshoeson <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
github-actions bot pushed a commit that referenced this issue Oct 16, 2024
@gotnoshoeson @github-actions
Fix for Issue 4762 (#4803)
8feac8f 
[Issue #4762 ](#4762)

- Creates an enum for passing context of `service_queues` being called
from within `on_initialize` and `on_idle` hooks. Uses a match statement
inside of `service_queues` to continue using the same code, but NOT
throw a `defensive` if being called within `on_idle` hook.
- The issue requested to not throw the `defensive` if being called from
within `on_idle` hook.
- Created the `ServiceQueuesContext` enum to pass as an argument of
`service_queues` when called within the `on_initialize` and `on_idle`
hooks. A match statement was added inside of `service_queues` to
continue to throw the defensive if called from `on_initialize` but NOT
throw the defensive if called from `on_idle`.

---------

Co-authored-by: gotnoshoeson <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
(cherry picked from commit 504edb1)
github-actions bot pushed a commit that referenced this issue Oct 16, 2024
@gotnoshoeson @github-actions
Fix for Issue 4762 (#4803)
2546d0d 
[Issue #4762 ](#4762)

- Creates an enum for passing context of `service_queues` being called
from within `on_initialize` and `on_idle` hooks. Uses a match statement
inside of `service_queues` to continue using the same code, but NOT
throw a `defensive` if being called within `on_idle` hook.
- The issue requested to not throw the `defensive` if being called from
within `on_idle` hook.
- Created the `ServiceQueuesContext` enum to pass as an argument of
`service_queues` when called within the `on_initialize` and `on_idle`
hooks. A match statement was added inside of `service_queues` to
continue to throw the defensive if called from `on_initialize` but NOT
throw the defensive if called from `on_idle`.

---------

Co-authored-by: gotnoshoeson <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
(cherry picked from commit 504edb1)
@ggwpez
Copy link
Member Author

ggwpez commented Oct 25, 2024

Done in #4803

@ggwpez ggwpez closed this as completed Oct 25, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Runtime / FRAME Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gotnoshoeson gotnoshoeson

Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.
Projects
Runtime / FRAME
Status: Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ggwpez @gotnoshoeson