TCP Keep Alive Settings

[chitownhart]

It doesn't look like the library is setting any TCP keep alive settings on the connection. I confirmed this by checking netstat -apno on my application. It appears the etcdctl command line utility sets pretty aggressive TCP keep alive settings by default.

  --keepalive-time=2s                       keepalive time for client connections
  --keepalive-timeout=6s                    keepalive timeout for client connections

I need similarly aggressive settings when using the etcd-cpp-apiv3 as I need to detect connection issues quickly so that I can failover to a different etcd node before my lease expires and continue to keep that lease alive. I'm not familiar with GRPC but it looks like it may have some TCP keep alive configuration settings. Maybe it's just configuration you need to expose and pass onto GRPC?

[chitownhart]

I was looking at GRPC and it looks like they are grpc arguments that would need to be set when you're creating the GRPC channel. Maybe something like this but use configurable variables...

etcd::Client::Client(std::string const & address, std::string const & load_balancer)
{
// create channels
std::string const addresses = etcd::detail::strip_and_resolve_addresses(address);
grpc::ChannelArguments grpc_args;
grpc_args.SetMaxSendMessageSize(std::numeric_limits::max());
grpc_args.SetMaxReceiveMessageSize(std::numeric_limits::max());
grpc_args.SetInt(GRPC_ARG_KEEPALIVE_TIME_MS, 2000);
grpc_args.SetInt(GRPC_ARG_KEEPALIVE_TIMEOUT_MS, 6000);
grpc_args.SetInt(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, 1);
std::shared_ptrgrpc::ChannelCredentials creds = grpc::InsecureChannelCredentials();
grpc_args.SetLoadBalancingPolicyNa

[chitownhart]

Final comment, makes we wonder if we need to expose a more generic way of passing through GRPC arguments -- not just these TCP keep alives?

[sighingnow]

Final comment, makes we wonder if we need to expose a more generic way of passing through GRPC arguments -- not just these TCP keep alives?

Would it be enough for your case if we expose an extra constructor which accepts an extra grpc::ChannelArguments argument ?

[chitownhart]

Yeah, I think that could work fine. You could probably drop the load_balancer parameter from these two new constructors as well and just make users responsible for calling SetLoadBalancingPolicyName, SetMaxMessageSize, and SetMaxReceiveMessageSize plus whatever other options they want to send like the TCP keep alive settings. It doesn't look like the ChannelArguments interface has getter methods to query the parameter values to see if they're set or not so I don't think these constructors could inject the current three settings if they're missing. It would be the responsibility of users using these new constructors to set ALL of the grpc parameters which shouldn't be an issue since it's just a few settings.

If you go that route, then I think the two new constructors would look like this.

Client
(
std::string const & etcd_url,
grpc::ChannelAgruments const & grcp_args
);

Client
(
std::string const & etcd_url,
grpc::ChannelArguments const & grcp_args,
std::string const & username,
std::string const & password
);

[sighingnow]

You could probably drop the load_balancer parameter from these two new constructors as well

Nice suggestions.

Will be fixed today or tomorrow.

[sighingnow]

Yeah, I think that could work fine. You could probably drop the load_balancer parameter from these two new constructors as well and just make users responsible for calling SetLoadBalancingPolicyName, SetMaxMessageSize, and SetMaxReceiveMessageSize plus whatever other options they want to send like the TCP keep alive settings. It doesn't look like the ChannelArguments interface has getter methods to query the parameter values to see if they're set or not so I don't think these constructors could inject the current three settings if they're missing. It would be the responsibility of users using these new constructors to set ALL of the grpc parameters which shouldn't be an issue since it's just a few settings.

If you go that route, then I think the two new constructors would look like this.

Client ( std::string const & etcd_url, grpc::ChannelAgruments const & grcp_args );

Client ( std::string const & etcd_url, grpc::ChannelArguments const & grcp_args, std::string const & username, std::string const & password );

The requested feature has been added to the master branch.