Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inefficient TCP connections use by memberlist transport #193

Open
pracucci opened this issue Jul 19, 2022 · 5 comments
Open

Inefficient TCP connections use by memberlist transport #193

pracucci opened this issue Jul 19, 2022 · 5 comments

Comments

@pracucci
Copy link
Contributor

We use a custom transport for memberlist, based on TCP protocol. The main reason why we use TCP is being able to transfer messages which are bigger than the maximum payload of an UDP packet (typically, slightly less than 64KB).

Currently, the TCP transport is implemented in an inefficient way with regards to TCP connection establishment. For every single packet a node needs to transfer to another node, the implementations creates a new TCP connection, writes the packet and then close the connection. See:

func (t *TCPTransport) writeTo(b []byte, addr string) error {

We should consider alternatives like:

  • Pros/cons of keeping long-lived TCP connections between nodes, and multiplexing multiple packets over the same connection
  • Using a mix of UDP and TCP, selecting the protocol based on the message size (in this case, TLS support wouldn't be available)
@pstibrany
Copy link
Member

pstibrany commented Jul 19, 2022

One alternative which we could explore is reusing gRPC connection and implement Packet (and perhaps Stream, if possible) operations on top of gRPC (as grpc methods). This would give us connection pooling, it would remove the need to configure another port, and it would reuse gRPC TLS settings.

@pstibrany
Copy link
Member

My reason to implement tcp transport the way it is, was to keep it simple, with no state on the connection. I agree it's not efficient, and it is time to revisit that decision.

@stevesg
Copy link
Contributor

stevesg commented Oct 4, 2023

I recently discovered another issue with this, though I'm unsure whether it's an immediate cause for concern - conntrack table utilization. These short lived connections live on in conntrack for some number of minutes.

A survey of a single node in one of our dev environments at Grafana showed that two thirds (~6000 of ~9000) of the conntrack table were TIME_WAIT dport=7946.

@seizethedave
Copy link
Contributor

Awesome. I would imagine persistent TCP conns would help quite a bit. UDP seems less desirable with intermittently flaky cloud networking, inability to do TLS, ...

@zalegrala
Copy link
Contributor

I thought it might be interesting to see the state of the QUIC protocol. Its been years since I looked, but there is a nice looking Go implementation with reasonable docs.

I copied tcp_transport.go and replaced some of the details for the handling. I'll see if I can test this soon, but curious what others think along these lines.

There are a few things to like about this protocol, though I have no hands-on experience with it. The wikipedia page has some good details as to what prompted the development and use of a new internet protocol.

For the memberlist use case, it might make a nice optional transport if we wanted to support more than one possibility. I've not considered what it would take to migrate from one memberlist transport to another.

Additionally, it appears that TLS is required, which could be a non-starter for some environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants