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

outbound: stack tests for idling out services #769

Closed
wants to merge 9 commits into from

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Dec 11, 2020

This branch adds new stack tests in the outbound crate testing behavior
around dropping idle services. I've added a new test utility for
wrapping a NewService to track the number of currently alive Service
instances created by that NewService, using an Arc. The tests can
then wrap the HTTP router stack with this wrapper, and make assertions
about how many services are alive.

I had to do a bit of stack surgery to fit the tracking layer in the right place.
Sorry in advance for any merge conflicts.

This branch adds two tests. One asserts that services are dropped after
the idle timeout when they are inactive, and another asserts that
services which are currently streaming response bodies are not idled
out.

The second of these tests fails on main, which is expected. We expect
PR #768 to fix this.

Depends on #765.

@hawkw hawkw changed the title [WIP] outbound: stack tests for idling out services outbound: stack tests for idling out services Dec 11, 2020
@hawkw hawkw marked this pull request as ready for review December 11, 2020 19:51
@hawkw hawkw requested a review from a team December 11, 2020 19:51
@hawkw hawkw self-assigned this Dec 11, 2020
@hawkw hawkw requested a review from olix0r December 11, 2020 19:51
}

pub fn stack_with_tcp_balancer<P, C, T, H, S, I>(
pub fn cache_accept<N, S, I>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to bikeshed the name of this function (and accept_with_tcp_balancer), I had to split these out to sneak in the tracking layer at the right point in the stack.

Copy link
Member

Choose a reason for hiding this comment

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

How about just cache for this? There's nothing to disambiguate this with, given that it's in the server module. And for accept_with_tcp_balancer, how about just stack?

Base automatically changed from eliza/test-balancer-creation to main December 11, 2020 20:33
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
...not the stack that's cloned into hyper's `serve_connection`. This
required slicing up the stack construction a bit to sneak the track
layer in at the correct place, sorry in advance for merge conflicts
@olix0r :P

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw
Copy link
Contributor Author

hawkw commented Dec 12, 2020

Closing this as we merged it directly into #768

@hawkw hawkw closed this Dec 12, 2020
@olix0r olix0r deleted the eliza/test-stacks-idle-out branch May 25, 2021 15:47
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.

2 participants