-
Notifications
You must be signed in to change notification settings - Fork 118
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
Timeout in connectAndBind method #7
base: master
Are you sure you want to change the base?
Conversation
@@ -219,7 +219,7 @@ public String connectAndBind(String host, int port, | |||
throw new IOException("Failed connecting"); | |||
} | |||
|
|||
conn = connFactory.createConnection(host, port); | |||
conn = connFactory.createConnection(host, port, timeout); | |||
logger.info("Connected"); | |||
|
|||
conn.setSoTimeout(getEnquireLinkTimer()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the case is we set timeout for 6000 millis
so the total connectAndBind can be more than 6000 millis.. why? because conn.setSoTimeout was set to enquireLinkTimer (of course the value is non 0)
You get what I mean?
I think the conn.setSoTimeout(...) should be set to the value of the remaining timeout taken by the createConnection... and when the state is bounded then the conn.setSoTimeout(...) should be set to enquireLinkTimer (set the timeout to normal)
It something like (CMIIW):
long startTime = System.currentTimeMillis();
conn = connFactory.createConnection(host, port, timeout);
long interval = System.currentTimeMillis() - startTime;
conn.setSoTimeout(timeout - interval);
So the total connectAndBind would be no longer than 6000 millis
... and when the state is bounded
conn.setSoTimeout(getEnquireLinkTimer());
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 24/05/2012 09:30, uudashr wrote:
@@ -219,7 +219,7 @@ public String connectAndBind(String host, int port,
throw new IOException("Failed connecting");
}
conn = connFactory.createConnection(host, port);
conn = connFactory.createConnection(host, port, timeout); logger.info("Connected"); conn.setSoTimeout(getEnquireLinkTimer());
If the case is we set timeout for 6000 millis
so the total connectAndBind can be more than 6000 millis.. why? because conn.setSoTimeout was set to enquireLinkTimer (of course the value is non 0)
You get what I mean?I think the conn.setSoTimeout(...) should be set to the value of the remaining timeout taken by the createConnection... and when the state is bounded then the conn.setSoTimeout(...) should be set to enquireLinkTimer (set the timeout to normal)
So the total connectAndBind would be no longer than 6000 millis
What do you think?
Reply to this email directly or view it on GitHub:
https://github.com/uudashr/jsmpp/pull/7/files#r873177Hello,
I think that the timeout which is defined in connectAndBind method is
totally different from those which is used in setSoTimeout.
The first one is for the establishment of the connection and replaces
the socket default binding timeout.
The second one is used for requests run on this socket.
When looking at the source code, I see that setEnquireLinkTimer is never
used, so I think that this second timeout is allways the same and is
independant from the first one.
Therefor, the time spent during the binding doesn't impact the
setSoTimeout which is allways the same.
That's my mind
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awsome.
the method signature:
String connectAndBind(String host, int port, BindParameter bindParam, long timeout)
at fist I was thinking about this question: "what is the timeout of method connectAndBind?" or "maximum time taken to connectAndBind?"
if you see, there is two things happen on connectAndBind
- connect (establish connection) -> this has timeout should has timeout
- bind, this happen after socket connection established -> this has timeout also
OR
so what you say is that we have different thing here.
- connect timeout (the timeout)
- bind timeout (soTimeout or enquireLinkTimer)
it means the total time taken for connectAndBind is connectTimeout + bindTimeout
so, let's put something to make it clean such as documentation that describe about this thing and maybe change the variable name of "timeout" to "connectTimeout". what do you think? after that I'll merge the code.
btw setEnquireLinkTimer use by user to change the timeout, in order to send the enquire link command
Could you please submit your pull request against https://github.com/opentelecoms-org/jsmpp to see if it builds in travis? |
Hello
As you invite me to do, I forked a new project embedding the timeout parameter in the connectAndBind method.
I hope all is correct, because I don't know how to test it from the new repository.
Regards
Claude