Skip to content
This repository has been archived by the owner on Oct 17, 2022. It is now read-only.

feat: Make all Worker channels metered #711

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

huitseeker
Copy link
Contributor

This is an extension of #682 to the worker

Copy link
Contributor

@gdanezis gdanezis left a comment

Choose a reason for hiding this comment

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

Awesome conversion!

Copy link
Contributor

@akichidis akichidis left a comment

Choose a reason for hiding this comment

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

Great work @huitseeker , thanks for doing this!

@@ -164,7 +162,7 @@ impl Synchronizer {

// Add the digest to the waiter.
let deliver = digest;
let (tx_cancel, rx_cancel) = channel(1);
let (tx_cancel, rx_cancel) = mpsc::channel(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah probably those could be oneshot channels? (not concern of this PR of course - but something to keep in mind)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They usually can't: this is a cancellation handler, and the branches of the tokio::select that GC and those that unpack items in waiting race each other. The GC might then try to cancel an item that's already delivered (and the delivery sinks the receiving end of the channel). With a oneshot channel, that may lead to a panic.

This is an extension of MystenLabs#682 to the worker
@huitseeker huitseeker enabled auto-merge (squash) August 8, 2022 13:31
@huitseeker huitseeker merged commit c3cc6ec into MystenLabs:main Aug 8, 2022
huitseeker added a commit to huitseeker/narwhal that referenced this pull request Aug 8, 2022
@huitseeker huitseeker mentioned this pull request Aug 8, 2022
huitseeker added a commit that referenced this pull request Aug 8, 2022
This is an extension of #682 to the worker
huitseeker added a commit that referenced this pull request Aug 12, 2022
This is an extension of #682 to the worker
mwtian pushed a commit to mwtian/sui that referenced this pull request Sep 30, 2022
@huitseeker huitseeker deleted the meter_worker branch January 21, 2023 13:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants