Skip to content

Thrift connection timeout on TcpClient.Connect() #1159

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

Closed
eddyzanda opened this issue Dec 17, 2014 · 3 comments
Closed

Thrift connection timeout on TcpClient.Connect() #1159

eddyzanda opened this issue Dec 17, 2014 · 3 comments

Comments

@eddyzanda
Copy link
Contributor

Hi @Mpdreamz,

We have an app that uses the thriftconnection to talk with elastic-server (es-sever).

We noticed that if es-server is unreachable the client takes long to timeout.

After some checks we changed the Thrift.Transport-socket-sconnect to "honor" the timeout property in the connection settings.

ConnectionSettings.SetTimeout(1000)

Modified the client connect code from:

namespace Elasticsearch.Net.Connection.Thrift.Transport
public class TSocket 
{
 public override void Open()
 {
  ...
  client.Connect(host, port);
 }

to:

public override void Open()
{
 ...
 var result = client.BeginConnect(host, port, null, null);    
 var sucess = result.AsyncWaitHandle.WaitOne(timeout);
 if (!sucess)
 {
  throw new Exception("Failed to connect");
 }
}

What are your thought on this?

@Mpdreamz
Copy link
Member

Good catch, it looks like TSocket already has a concept for timeout and we should pass our time when we create one here:

https://github.com/elasticsearch/elasticsearch-net/blob/develop/src/Connections/Elasticsearch.Net.Connection.Thrift/ThriftConnection.cs#L46

Hopefully this should suffice, would love to hear your confirmation!

@eddyzanda
Copy link
Contributor Author

Thanks for your reply.

Agreed it sounds better to pass it through upon creation time.

Just calling your attention to a "similar" thing is being done a couple of lines bellow

tsocket.Timeout = this._connectionSettings.Timeout;

which sets the tsocket timeout.

public int Timeout
{
 set { client.ReceiveTimeout = client.SendTimeout = timeout = value; }
}

eddyzanda added a commit to eddyzanda/elasticsearch-net that referenced this issue Dec 18, 2014
Changed the Tsocket to timeout using the property from the
connection-settings.
gmarz added a commit that referenced this issue Dec 18, 2014
@gmarz
Copy link
Contributor

gmarz commented Dec 23, 2014

Closed via #1163

@gmarz gmarz closed this as completed Dec 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants