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

MessageQueue: unknit permanently overweight books #13528

Merged
merged 6 commits into from
Mar 7, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Mar 3, 2023

Discovered in #13502

A book with only permanently overweight messages should be unkit from the ready ring since there can be no progress on it. This does currently not happen since permanently overweight messages are not counted as "processed" and therefore don't increase the "total_processed" counter which would trigger this.

This is only a problem when the next and only message that is processed is overweight. Eventually this should resolve itself when another non-overweight message is enqueued and processed. But for correctness it should be unknitted, since we have the invariant that all Books in the ReadyRing are ready.

TODO:

  • re-benchmark
  • Tests

A book with only permanently overweight messages should be unkit
from the ready ring. This does currently not happen since perm.
overweight messages are not counted as "processed" and therefore
not increase the "total_processed" counter.

This is only a problem when the next and only message that is
processed is overweight. Eventually this should resolve itself
when another non-overweight message is enqueued and processed.
But for correctness it should be unknitted.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez added the A3-in_progress Pull request is in progress. No review needed at this stage. label Mar 3, 2023
@ggwpez
Copy link
Member Author

ggwpez commented Mar 3, 2023

bot bench $ pallet dev pallet-message-queue

@command-bot
Copy link

command-bot bot commented Mar 3, 2023

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2480007 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet-message-queue. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-8965078e-d40e-49ed-b9bc-6faddab58a9c to cancel this command or bot cancel to cancel all commands in this pull request.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez added 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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Mar 3, 2023
@gavofyork gavofyork added T1-runtime This PR/Issue is related to the topic “runtime”. and removed D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Mar 3, 2023
@command-bot
Copy link

command-bot bot commented Mar 3, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet-message-queue has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2480007 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2480007/artifacts/download.

@ggwpez
Copy link
Member Author

ggwpez commented Mar 6, 2023

bot bench $ pallet dev pallet-message-queue

@command-bot
Copy link

command-bot bot commented Mar 6, 2023

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2492083 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet-message-queue. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 25-bf826007-2bcb-44e8-85ca-57abd3200adb to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Mar 6, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet-message-queue has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2492083 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2492083/artifacts/download.

@ggwpez
Copy link
Member Author

ggwpez commented Mar 7, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 306382b into master Mar 7, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-unknit-perm-overweight branch March 7, 2023 11:31
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
* Unknit permanently overweight books

A book with only permanently overweight messages should be unkit
from the ready ring. This does currently not happen since perm.
overweight messages are not counted as "processed" and therefore
not increase the "total_processed" counter.

This is only a problem when the next and only message that is
processed is overweight. Eventually this should resolve itself
when another non-overweight message is enqueued and processed.
But for correctness it should be unknitted.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Add tests

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* fmt

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* One more tests

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet-message-queue

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Unknit permanently overweight books

A book with only permanently overweight messages should be unkit
from the ready ring. This does currently not happen since perm.
overweight messages are not counted as "processed" and therefore
not increase the "total_processed" counter.

This is only a problem when the next and only message that is
processed is overweight. Eventually this should resolve itself
when another non-overweight message is enqueued and processed.
But for correctness it should be unknitted.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Add tests

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* fmt

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* One more tests

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet-message-queue

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants