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

hedge: don't reserve slots for hedged requests #472

Merged
merged 1 commit into from
Oct 2, 2020

Conversation

hdevalence
Copy link
Contributor

Hedge wraps S: Service + Clone and is essentially implemented as
Select<S, Delay<S>>, whose call implementation creates two response
futures and selects the one which resolves first. The second response
future is obtained from Delay<S>::call, which waits for some time
before calling S::call.

The current implementation of Delay<S>::poll_ready calls
S::poll_ready unconditionally. However, the S::call corresponding
to that poll_ready only happens after much delay, if ever. This
interacts badly with Buffer, which uses poll_ready to reserve a
channel slot. When using Hedge with a Buffer'd service, the inner
Buffer::poll_ready is called twice, but only one slot is used
immediately. This causes exhaustion of all of the Buffer slots.

Instead, return Poll::Ready(Ok(())) from Delay::poll_ready and use a
Oneshot inside of the Delay future.

`Hedge` wraps `S: Service + Clone` and is essentially implemented as
`Select<S, Delay<S>>`, whose `call` implementation creates two response
futures and selects the one which resolves first.  The second response
future is obtained from `Delay<S>::call`, which waits for some time
before calling `S::call`.

However, the current implementation of `Delay<S>::poll_ready` calls
`S::poll_ready` unconditionally.  However, the `S::call` corresponding
to that `poll_ready` only happens after much delay, if ever.  This
interacts badly with `Buffer`, which uses `poll_ready` to reserve a
channel slot.  When using `Hedge` with a `Buffer`'d service, the inner
`Buffer::poll_ready` is called twice, but only one slot is used
immediately.  This causes exhaustion of all of the `Buffer` slots.

Instead, return `Poll::Ready(Ok(()))` from `Delay::poll_ready` and use a
`Oneshot` inside of the `Delay` future.
@hawkw
Copy link
Member

hawkw commented Sep 23, 2020

Just to confirm, this fixes an issue in Zebra, correct?

@hdevalence
Copy link
Contributor Author

Yes, or it would, if the code that uses it was merged. The Zebra context is here: ZcashFoundation/zebra#1089

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

this seems right to me!

@LucioFranco LucioFranco merged commit 1a84543 into tower-rs:master Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants