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

Taskmanager: Remove clean_shutdown #10314

Merged
merged 1 commit into from
Nov 19, 2021
Merged

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Nov 19, 2021

There is no reason for this function, tokio already blocks automatically until all tasks are ended.
Another reason to remove this feature is mpsc_background_tasks unbounded channel. Recently this
channel was reporting too many unprocessed elements. We assume that this was a result of a lot of
very shot lived tasks that somehow flooded this channel.

polkadot companion: paritytech/polkadot#4336

skip check-dependent-cumulus

There is no reason for this function, tokio already blocks automatically until all tasks are ended.
Another reason to remove this feature is `mpsc_background_tasks` unbounded channel. Recently this
channel was reporting too many unprocessed elements. We assume that this was a result of a lot of
very shot lived tasks that somehow flooded this channel.
@bkchr bkchr 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Nov 19, 2021
@bkchr bkchr requested a review from ordian November 19, 2021 09:37
@ordian
Copy link
Member

ordian commented Nov 19, 2021

We assume that this was a result of a lot of very shot lived tasks that somehow flooded this channel.

I don't quite get why mpsc_background_tasks would grow indefinitely in this case

Also, do you think a burn-in is justified here?

@bkchr
Copy link
Member Author

bkchr commented Nov 19, 2021

I don't quite get why mpsc_background_tasks would grow indefinitely in this case

Me neither. I checked the code that is called there and we don't use anything self build, most of the stuff is coming from the futures crate directly. Basically it maps down to a FuturesUnordered. However, as we don't require this, I don't see any reason for wasting time on this.

Also, do you think a burn-in is justified here?

This is nothing critical. This stuff was only used to ensure that all tasks are stopped before the node shuts down, nothing more.

@bkchr
Copy link
Member Author

bkchr commented Nov 19, 2021

bot merge

@paritytech-processbot
Copy link

Error: It's not possible to meet the minimal approval count of 1 in paritytech/polkadot#4336

@bkchr bkchr merged commit 3cf5449 into master Nov 19, 2021
@bkchr bkchr deleted the bkchr-clean-shutdown-remove branch November 19, 2021 20:30
bkchr added a commit to paritytech/polkadot that referenced this pull request Nov 19, 2021
* Companion for Taskmanager: Remove `clean_shutdown`

paritytech/substrate#10314

* Update Substrate

* Remove warning
@bkchr bkchr added this to the Polkadot v0.9.13 milestone Nov 25, 2021
bkchr added a commit to paritytech/polkadot that referenced this pull request Nov 30, 2021
* Companion for Taskmanager: Remove `clean_shutdown`

paritytech/substrate#10314

* Update Substrate

* Remove warning
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
There is no reason for this function, tokio already blocks automatically until all tasks are ended.
Another reason to remove this feature is `mpsc_background_tasks` unbounded channel. Recently this
channel was reporting too many unprocessed elements. We assume that this was a result of a lot of
very shot lived tasks that somehow flooded this channel.
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
There is no reason for this function, tokio already blocks automatically until all tasks are ended.
Another reason to remove this feature is `mpsc_background_tasks` unbounded channel. Recently this
channel was reporting too many unprocessed elements. We assume that this was a result of a lot of
very shot lived tasks that somehow flooded this channel.
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants