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.

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

Document use of future::timeout in combination with channels #411

Open
yoshuawuyts opened this issue Oct 29, 2019 · 1 comment
Open

Document use of future::timeout in combination with channels #411

yoshuawuyts opened this issue Oct 29, 2019 · 1 comment
Labels
documentation Improvements or additions to documentation

Comments

@yoshuawuyts
Copy link
Contributor

As shared here, channels could potentially deadlock if the capacity isn't large enough:

let (s, r) = channel(1);

s.send(1).await;
s.send(2).await; // this will hang indefinitely

Instead we should probably mention that by using future::timeout this can be prevented from hanging indefinitely:

let (s, r) = channel(1);

future::timeout(s.send(1), Duration::from_secs(1).await);
future::timeout(s.send(2), Duration::from_secs(1).await);

Perhaps we could even mention in the docs that folks should probably default to something spacious such as 256 or 1024 messages in case of doubt. In general we probably shouldn't assume people have intuition about channels, and nudging them towards some safer default choices (because unlike with unbounded channels they have to choose) wouldn't be a bad idea probably (:

@yoshuawuyts yoshuawuyts added the documentation Improvements or additions to documentation label Oct 29, 2019
@Matthias247
Copy link

Perhaps we could even mention in the docs that folks should probably default to something spacious such as 256 or 1024 messages in case of doubt. In general we probably shouldn't assume people have intuition about channels, and nudging them towards some safer default choices (because unlike with unbounded channels they have to choose) wouldn't be a bad idea probably (:

Please not - that is the wrong advice. The outcome of this is that the system will then just deadlock under heavy load instead of early on. This will be even harder to detect, diagnose and fix. And it will exactly happen when you need it at least: In production under stress. Channels should be be used in a fashion where they can't deadlock independent of the size. As the Uber Go Styleguide mentions: Channels should be sized by default of size 0 or 1. All other sizes can be used to improve throughput by helping to buffer across some switching latencies, but they shouldn't be used to fix architecture. They will also in doubt increase tail latencies.

The issues should be fixed with having the right architecture, which e.g. means producer and consumer shouldn't run on the same thread/task, and there should not be cyclic dependencies. If there are cyclic dependencies (e.g. a return channel) then that channel should be used in a very limited and deterministic fashion. E.g. it might be used exactly once and have a capacity of 1 (oneshot channels are good for this). For other use-cases where producer and consumer are on different threads rendezvous channels (size 0) guarantee the highest chance for correctness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants