protobuf-net / protobuf-net.Grpc

GRPC bindings for protobuf-net and grpc-dotnet
Other
846 stars 106 forks source link

[Question] reject request without paying deserialization cost #311

Open vrnmthr opened 8 months ago

vrnmthr commented 8 months ago

Context: I have a grpc server that supports high & low priority traffic. If it gets too many requests I want to reject low priority traffic (return ResourceExhausted). Low pri traffic is identifiable by a priority int that is sent as a RequestHeader from the client. I am on ASP.NET core

However, I would be able to like to do this without deserializing the message so that my server can't get knocked over just by sending it a ton of traffic. Doing this at the load balancer level is a bit difficult for us right now.

From what I've seen online, the right way to go seems to be: 1) pass the priority in as a header 2) implement a custom marshaller

I was wondering if there is built in support to do something like this without requiring a custom marshaller? Maybe an interceptor only solution? I imagine that this is a relatively common use case.

mgravell commented 8 months ago

You can't implement this purely in a marshaller, as the marshaller only has access to a DeserializationContext, which does not expose anything request-related like headers. I wonder if @JamesNK might have thoughts on this - might need support higher up in the server stack (although I can help there too, now!)

vrnmthr commented 8 months ago

@JamesNK what do you think?

Someone seems to have put an interceptor + marshaller recipe for java here: https://groups.google.com/g/grpc-io/c/lrSj2iuMx3A. Do you know if we have these equivalents in protobuf-net?

vrnmthr commented 8 months ago

I am also flexible in terms of how we communicate the priority, if there is a different non-header way that makes it easier.

JamesNK commented 8 months ago

You need to have middleware before gRPC. Maybe you could use ASP.NET Core rate limiting?

https://learn.microsoft.com/en-us/aspnet/core/performance/rate-limit?view=aspnetcore-7.0

mgravell commented 8 months ago

@JamesNK what do you think?

Someone seems to have put an interceptor + marshaller recipe for java here: https://groups.google.com/g/grpc-io/c/lrSj2iuMx3A. Do you know if we have these equivalents in protobuf-net?

The question isn't about "in protobuf-net" - protobuf-net.Grpc sits on top of the Goggle/Microsoft gRPC server implementation - and the Interceptor implementation there: has already done deserialization before it gets invoked.

mgravell commented 8 months ago

You need to have middleware before gRPC. Maybe you could use ASP.NET Core rate limiting?

I agree that I don't think anything exists today, but: should something better exist here? perhaps a new method on Intereceptor like public virtual bool AcceptRequest(ServerCallContext context) => true; that gets called before other things?

JamesNK commented 8 months ago

Middleware is a shared layer between all ASP.NET Core frameworks, and keeping the logic there and recommending rate-limiting middleware for this task reduces duplication of code and concepts.

Perhaps having access to ServerCallContext in AcceptRequest would make accessing the gRPC method name easier. However, the endpoint is already matched with routing, and the gRPC endpoint has metadata that can be used to access information about which method is being called. That information can be fetched from the HttpContext.

public Task Invoke(HttpContext context)
{
    var endpoint = context.GetEndpoint();
    var grpcMethodMetadata = endpoint.Metadata.GetMetadata<GrpcMethodMetadata>();
    // ...
}
vrnmthr commented 3 months ago

@JamesNK if I do use this middleware approach, how can I return a grpc error code? I can only return an http status from within a middleware. How do these get mapped to grpc errors, and what will the client see?

JamesNK commented 3 months ago

A grpc error is just a trailer. You can add a trailer to the HttpResponse.

Alternatively you can return a regular HTTP status code and clients will map it to a gRPC status code.

vrnmthr commented 3 months ago

I got this to mostly work with the following code in my custom middleware:

public async Task Invoke(HttpContext httpContext)
{
    if (ShouldRatelimit())
    {
        // return resource exhausted
        httpContext.Response.Headers.ContentType = "application/grpc";
        httpContext.Response.Headers.GrpcMessage = "exceeded queue count";
        httpContext.Response.Headers.GrpcStatus = ((int)StatusCode.ResourceExhausted).ToString();
        httpContext.Response.StatusCode = (int)HttpStatusCode.OK;
        return;
    }

    await _next(httpContext);
}

This mostly works as expected, but I get these errors intermittently from my load test client:

ERROR: server 'http://localhost:23400/V3/grpc' is not online. Unable to connect. e=Status(StatusCode="ResourceExhausted", Detail="Error starting gRPC call. HttpRequestException: An error occurred while sending the request. IOException: The request was aborted. Http2StreamException: The HTTP/2 server reset the stream. HTTP/2 error code 'ENHANCE_YOUR_CALM' (0xb).", DebugException="System.Net.Http.HttpRequestException: An error occurred while sending the request.")

@JamesNK I read your response here about ENHANCE_YOUR_CALM: https://github.com/grpc/grpc-dotnet/issues/2010#issuecomment-1373049007. Why is my grpc call not "gracefully completed" in this scenario? We are using a unary call, so there is no streaming involved. The client is calling the following auto-generated function:

[GeneratedCode("grpc_csharp_plugin", null)]
public virtual AsyncUnaryCall<TranslationResponse> TranslateAsync(TranslationRequest request, CallOptions options)
{
    return base.CallInvoker.AsyncUnaryCall(__Method_Translate, null, options, request);
}