Comments (6)
I'm considering a new approach to the DNS timeout problem. The design looks something like the diagram below.
What I have in mind is a new DnsTimeout
middleware wrapped around the LoadBalancedChannel
. I'm imagining it would work like so:
- If the underlying service is not ready, it checks whether our
GrpcServiceProbe
currently has any discovered endpoints. If it does, the lack of readiness is because of something else and we returnPending
. If it's empty, then we'll need to wait until it does have a discovered endpoint or until we reach a timeout. - The background probe and the
DnsTimeout
middleware need to communicate somehow. I was planning on using a shared AtomicBool (is_empty
) for this, but keen to hear other ideas. - When
DnsTimeout
times outpoll_ready()
will returnReady(Ok)
and the nextcall()
will return a timeout error.
@conradludgate (or anyone else) what do you think?
from ginepro.
I'm going to take some time to fully digest what ginepro/tower/tonic are currently doing under the hood. Then I'll have a think about how this dnstimeout fits in
from ginepro.
Agreed!
After a few attempts of using the dead connection we can remove it and continue using the 2 remaining connections
I had to have a look, but I'm guessing this is because tonic's Reconnect
will error from poll_ready()
iff the connection broke and then the subsequent reconnection attempt also failed?
So, services can be removed from the balancer for one of two reasons:
- Evicted by our GrpcServiceProbe (DNS stopped returning it).
- Failing to become ready, most likely the reason above.
Cool. So yeah, either way we can end up with nothing to load balance across.
Of course, for DNS local to a kubernetes cluster, I would hope those DNS latencies would be low, so you could set the timeout for the DNS to be low but the timeout for the connection to be higher. Is that your understanding too?
Yep. I mean, how long DNS vs TCP + TLS vs actual requests will take can vary a lot, which is why separate timeouts are useful. E.g. we might be happy to wait for a really long request which can take 10s P99, but we wouldn't want to wait that long just for DNS or TCP + TLS. We already have (well, will soon have) a timeout for the TCP connection, so DNS seems like the last piece of the puzzle.
from ginepro.
Ok, just to confirm that I have a clear picture of what the issue is:
Let's say we have 3 connections in our balance queue, but 1 of them has gone offline. We don't need to wait for DNS in this case. After a few attempts of using the dead connection we can remove it and continue using the 2 remaining connections (not sure if tower::Balance does this. TBC).
Let's say those 2 connections go down, we now have 0 connections in our balance queue. Therefore we need to wait for DNS to get new connections. This is also the init case. When this happens, we want to timeout early waiting for DNS, which would be a different timeout from the unary request round trip.
Of course, for DNS local to a kubernetes cluster, I would hope those DNS latencies would be low, so you could set the timeout for the DNS to be low but the timeout for the connection to be higher. Is that your understanding too?
from ginepro.
After a few attempts of using the dead connection we can remove it
https://docs.rs/tower/0.4.13/tower/ready_cache/cache/struct.ReadyCache.html
If an error is returned, this indicats that the server failed and has been removed from the cache entirely.
from ginepro.
Cool. I'm all caught up then. I'll have a think about yours and some other solutions then.
In the end I think we already might have to re-implement some of tonic for #38 so I think we can move the dnstimeout further download the chain (in the Discover layer). But we'll still somehow have to surface the number of services registered to this layer
from ginepro.
Related Issues (7)
- RUSTSEC-2021-0073: Conversion from `prost_types::Timestamp` to `SystemTime` can cause an overflow and panic
- wait for an initial resolution to succeed before fully constructing `LoadBalancedChannel`
- Consider means of configuring Endpoints
- ginepro does not load 'balance' HOT 4
- Problems updating from 0.5.1 to 0.5.2 HOT 1
- Unexpected blocked when custom lookup service returns ipv6 addresses
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ginepro.