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

fix: Make metered channel accounting take unused dropped Permits into account #849

Merged
merged 3 commits into from
Aug 25, 2022

Conversation

huitseeker
Copy link
Contributor

@huitseeker huitseeker commented Aug 25, 2022

Context

We introduced metered channels in #682. Then we started using mpsc::Permit in #724 to implement composable backpressure.

The issue

Permits increase the occupancy metric of their underlying channel when acquired. They decrease that occupancy when they are used to send an element that's received. They do not decrease this occupancy when the permit is dropped without being utilized to send anything.

This leads to the following graphs on precisely the three channels which in #724 were made to use permits:
Screen Shot 2022-08-25 at 11 40 29 AM

For context, those channels have a bounded capacity of 1000 elements (due to the underlying use of a tokio::mpsc::channel). They show occupancy far above that, which constitutes the bug.

The fix

We first introduce a (failing) test test_reserve_and_drop that shows the issue, and fix it by making permits from metered channels decrease the occupancy of their underlying channel when they are dropped without having sent an element.

@huitseeker huitseeker marked this pull request as ready for review August 25, 2022 15:44
@huitseeker huitseeker requested a review from asonnino as a code owner August 25, 2022 15:44
@huitseeker huitseeker merged commit 65ef540 into MystenLabs:main Aug 25, 2022
@huitseeker huitseeker deleted the maintenance5 branch January 21, 2023 13:36
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.

2 participants