grpc / grpc-dotnet

gRPC for .NET
Apache License 2.0
4.19k stars 769 forks source link

How to obtain the default HttpHandler in order to only influence keepalive settings #1950

Open OkkeHendriks opened 1 year ago

OkkeHendriks commented 1 year ago

Hello,

I want to enable HTTP/2 keep alive for a gRPC client. I believe that I should supply the relevant settings via the GrpcChannelOptions.HttpHandler property.

My question is, how should I construct a default HttpMessageHandler, which normally happens here.

I only want to influence the keepalive settings, the rest should be the gRPC defaults which are used internally.

Is there something I am missing here or should those default be exposed somehow?

OkkeHendriks commented 1 year ago

I now implemented it as follows, but I only set EnableMultipleHttp2Connections because I saw this being done here.

    /// Apply default HTTP2 gRPC keep-alive settings. The time between keep-alive pings is ten seconds, and the timeout
    /// per ping is five seconds, the default `HttpKeepAlivePingPolicy` is `HttpKeepAlivePingPolicy.WithActiveRequests`.
    let withDefaultKeepAlive (channelOptions: GrpcChannelOptions) =
        let handler = new System.Net.Http.SocketsHttpHandler ()
        // Minimal value is 10s, lower values are automatically increased to 10s.
        handler.KeepAlivePingDelay <- TimeSpan.FromSeconds 10
        // Time to wait for a ping reply before the connection is closed. Applications should ensure keep-alive timeout
        // is at least multiple times the round-trip time to allow for lost packets and TCP retransmits.
        handler.KeepAlivePingTimeout <- TimeSpan.FromSeconds 5
        // Only send pings when there are active RPCs (or streams).
        handler.KeepAlivePingPolicy <- HttpKeepAlivePingPolicy.WithActiveRequests

        // `EnableMultipleHttp2Connections` is also enabled by the default internal gRPC HttpHandler.
        handler.EnableMultipleHttp2Connections <- true

        channelOptions.HttpHandler <- handler
        channelOptions.DisposeHttpClient <- true

        channelOptions

I would rather not have to depend on this manual copy of the default settings.

JamesNK commented 1 year ago

HttpHandler doesn't have a good way to compose your settings with the default settings. What you're doing, creating a new SocketsHttpHandler and then setting everything on it, is the best solution.

OkkeHendriks commented 1 year ago

Thank you for the confirmation :).

Should there be an option on the channel level to be able to set keep alive settings? Maybe via the channel options provided via the GrpcChannel.ForAddress(...) call?

Doing this via this http handler feels very 'low-level' to me. I also think that enabling HTTP keep alives would (should?) be very common. Especially because gRPC supports long lived streams and keep alives are the only way to timely detect certain connection failures. I also see this going wrong at my place of work, people often do not realize that enabling the keep alive settings is required. It is assumed that a channel already 'reliably' detects connection failures.

Side question: using the Grpc.Net.Client package I still have access to Grpc.Core.Channel(...) which does support setting the keepalive options via string based ChannelOption's. Should this way of creating a channel be used at all?

OkkeHendriks commented 1 year ago

There is a similar discussion about channel options at https://github.com/grpc/grpc-dotnet/issues/1842.

JamesNK commented 1 year ago

The problem is a lot of these APIs are only available on certain HttpHandler implementations. What should happen if the handler doesn't support keep alive? Throw an error? Silently ignore the setting?

OkkeHendriks commented 1 year ago

Channel options

Does anyone know how/if this worked with the old Grpc.Net.Core? There you are able to set options on a channel, via Grpc.Core.ChannelOptions:

Defines names of most commonly used channel options. Other supported options names can be found in grpc_types.h (GRPCARG* definitions)

Some of them are predefined in that class some you can only set via a custom Grpc.Core.ChannelOption(...). I do not know if all options will actually always work? On all platforms and .NET versions? Grpc.Core.ChannelOption documentation says:

Channel option specified when creating a channel. Corresponds to grpc_channel_args from grpc/grpc.h. Commonly used channel option names are defined in ChannelOptions, but any of the GRPCARG* channel options names defined in grpc_types.h can be used.

How was this previously implemented? Do all options always work? Only as long as you do not change the default HttpHandler?

Default HttpHandler

I understand that some of the options might not be applicable if the underlying HttpHandler is non-default and does not support them. So what is the idea? Expose via GrpcChannelOptions only handler/client independent options (do those actually exist?) and still enable users to set non-generic options via a custom HttpHandler only?

My original question was if it is possible to obtain the default HttpHandler. As the internals also construct this handler at some point, based on some branching. Wouldn't it be as simple as exposing this default HttpHandler construction publicly?

Then users could only overwrite the settings, on that default handler, which they actively want to change.

OkkeHendriks commented 1 year ago

Side question: using the Grpc.Net.Client package I still have access to Grpc.Core.Channel(...) which does support setting the keepalive options via string based ChannelOption's. Should this way of creating a channel be used at all?

Answering my own side question, I had a reference to the old Grpc.Core package via another package. That is why those functions actually existed in the Grpc.Core namespace. If only referencing the new Grpc.Net.Client and/or Grpc.AspNetCore.Server package(s) these are not 'available'.