Expand director API to allow for connection management #16
Conversation
proxy/examples_test.go
Outdated
| if strings.HasPrefix(method, "/com.example.internal.") { | ||
| return nil, nil, grpc.Errorf(codes.Unimplemented, "Unknown method") | ||
| } | ||
| md, ok := metadata.FromContext(ctx) |
README.md
Outdated
| [](https://travis-ci.org/mwitkow/grpc-proxy) | ||
| [](https://goreportcard.com/report/github.com/mwitkow/grpc-proxy) | ||
| [](https://godoc.org/github.com/mwitkow/grpc-proxy) | ||
| [](https://travis-ci.org/vgough/grpc-proxy) |
There was a problem hiding this comment.
hmm.. this was not part of the PR I initially sent. Looks like it keeps getting updated with the state of my fork. I'll see what I can do.
|
Updated examples, removed status link changes. |
|
Why is this needed? The naive implementation re-creates a new For example: Am I missing something? |
|
@mwitkow the problem is one of scale - see #15. Imagine that you potentially contact 10s of thousands of endpoints. Your example pool has a close-all function, but that's not safe to use without disconnecting live connections. How would you implement a pool which keeps connections around for T seconds after last use, or at most N connections? In order to do that, you need to know when the handler is done using the connection, so that you don't end up disconnecting live streaming connections. If the director were to return an interface, it would be possible wrap the grpc connection in something that tracks lifespan. Since we're returning a struct, I went the route of adding an explicit release method. |
|
vgough, I understand the use case, but this is not a problem with the grpcproxy code, but instead with the fact that We had a similar problem in other places (e.g. ephemeral connections to ip addresses), and we solved it by having a piece of middleware that accounted for in flight RPCs and cleared stuff from the pool. I'd rather not change the interface of grpcproxy atm, as I am planning for it to go through a stabilization phase. |
|
Regardless, we need go get the metadata proxying code in here, can we do
that in another PR?
…On Fri, Aug 25, 2017 at 3:33 AM, Michal Witkowski ***@***.***> wrote:
vgough, I understand the use case, but this is not a problem with the
grpcproxy code, but instead with the fact that grpc.Conn is not an
interface.
We had a similar problem in other places (e.g. ephemeral connections to ip
addresses), and we solved it by having a piece of middleware that accounted
for in flight RPCs and cleared stuff from the pool.
I'd rather not change the interface of grpcproxy atm, as I am planning for
it to go through a stabilization phase.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AECGvD1JqFgP2sg82ajybX-0lO91sB8Xks5sbni8gaJpZM4N75XC>
.
|
|
Michal, sounds like you're thinking that a connection-managing proxy would implement a new Seems like pretty basic functionality, knowing when TransparentHandler is done with a resource taken from StreamDirector. Would you include this wrapper in grpcproxy? If so, then I'm willing to redo this PR to add the new interfaces. |
|
Closing this, my fork is diverging, not expecting to integrate changes. |
Fixes #15 , allowing director to decide how to deal with connections - either by closing them when they're not used, or allowing the director to track how long they've been inactive.
Adds forwarding metadata, and updates metadata functions to no longer use the deprecated functions.