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

Socket probe transport doesn't accept / handle incoming connections #21

Closed
Aaronontheweb opened this issue Mar 14, 2019 · 4 comments
Closed
Labels
bug Something isn't working

Comments

@Aaronontheweb
Copy link
Member

Given how this is implemented:

_socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
_socket.Bind(new IPEndPoint(IPAddress.IPv6Any, Settings.Port));
_socket.Listen(10);

We never actually handle the incoming socket requests, thus the liveness probe will eventually fail given enough time. Need to actually handle the socket request and verify it by sending back some trivial piece of data.

Can you make this fix and verify it via a TCP integration test @izavala ?

@Aaronontheweb Aaronontheweb added the bug Something isn't working label Mar 14, 2019
@Aaronontheweb Aaronontheweb added this to the v0.2.0 Release milestone Mar 14, 2019
@izavala
Copy link
Contributor

izavala commented Mar 14, 2019

Currently the issue that we are seeing with the current setup is:

  • Sockets are being opened and we are binding to them, but we are not accepting the connection.
  • This will hold the resource and will cause the sockets to timeout and close.
  • Once a connection is made, we are not acknowledging that we have handled the connection request.

How to resolve the issue:

  • Will implement Akka.io to bind to the Tcp port.
  • Once we receive a Tcp.connected message we will respond with a small message.
  • After sending the message we will want to close the connection and get back to the listening state.

@izavala
Copy link
Contributor

izavala commented Mar 14, 2019

It looks like using Akka.io may complicate this process as SocketStatusTransport is not an actor and we will not be able to pass it through as an ActorRef.

Using the AccpetAsync socket function and handling these request through Tasks may be an easier implementation.

@izavala
Copy link
Contributor

izavala commented Mar 14, 2019

From looking at the implementation of LivenessTransportActor, it waits for the SocketStatusTransport to open the connection and respond with a TransporWriteStatus(True) message if no exceptions are encountered.

After we have an initial bind with the socket, we would then create a temporary connection to echo the message back and close it immediately after to accept queued connections.

Adding
var handler = await _socket.AcceptAsync();
byte[] msg = Encoding.ASCII.GetBytes("Live");
handler.Send(msg);
handler.Shutdown(SocketShutdown.Both);
handler.Close();

between the if statement and the exception would allow the connection request to come through and close once it has been acknowledged?

https://github.com/izavala/akkadotnet-healthcheck/blob/85ef553f8ef5c98f1dee3a081003c75668ce93da/src/Akka.HealthCheck/Transports/Sockets/SocketStatusTransport.cs#L33-L40

@Aaronontheweb
Copy link
Member Author

closed via #25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants