gRPC server: add ability to reject methods early, before reading the request into memory #377
Conversation
server/server.go
Outdated
| @@ -118,6 +118,7 @@ type Config struct { | |||
| GRPCServerTimeout time.Duration `yaml:"grpc_server_keepalive_timeout"` | |||
| GRPCServerMinTimeBetweenPings time.Duration `yaml:"grpc_server_min_time_between_pings"` | |||
| GRPCServerPingWithoutStreamAllowed bool `yaml:"grpc_server_ping_without_stream_allowed"` | |||
| GRPCServerMaxInflightRequests int `yaml:"grpc_server_max_inflight_requests"` | |||
There was a problem hiding this comment.
I'm not super convinced of having a global config. I think in practice it's more useful to let the upstream project to have control over it (e.g. limit only specific methods). Have you considered designing so that we just configure a set of 0+ limiters, each limiter being able to limit a configurable set of methods? I'm asking because in Mimir ingester I would like to limit only the push gRPC and not every gRPC call with a "global" limiter.
There was a problem hiding this comment.
Have you considered designing so that we just configure a set of 0+ limiters, each limiter being able to limit a configurable set of methods? I'm asking because in Mimir ingester I would like to limit only the push gRPC and not every gRPC call with a "global" limiter.
This PR allows you to do exactly that already. There's global inflight limit, which I think can be useful but can also be disabled, and there's also a possibility to limit specific methods by using GrpcMethodLimiter. I imagine that Mimir would install its own GrpcMethodLimiter to implement ingesters's inflight limit.
There was a problem hiding this comment.
I would just remove the global one and let the upstream project to control it via GrpcMethodLimiter. WDYT?
There was a problem hiding this comment.
I would just remove the global one and let the upstream project to control it via
GrpcMethodLimiter. WDYT?
OK, let's do that.
There was a problem hiding this comment.
I've removed support for max inflight requests in gRPC server, and only added limiting via GrpcInflightMethodLimiter. Please take a look again.
…rpcInflightMethodLimiter.
pstibrany
changed the title
What this PR does:
This PR introduces adds a way to reject calls to gRPC methods early, right after gRPC server receives request headers, but before reading the request body and starting a goroutine.
This PR is generalization of Mimir's grafana/mimir#5976. Description in that PR explains this approach and why it works.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]