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.

Sign up for GitHub

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

Jump to bottom

Timeout in connectAndBind method #7

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+10 −1
Open

Timeout in connectAndBind method #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
707f6f4
Add the timeout parameter in connectAndBind method, createConnection,…
Yaude May 19, 2012
aa1ab25
Ajout de l'interface createConnection(host, port, timeout)
Yaude May 19, 2012
d2369bd
Add socket creation with timeout value
Yaude May 19, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jsmpp/src/main/java/org/jsmpp/session/SMPPSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Owner

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?

Copy link
Author

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#r873177

Hello,

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

Copy link
Owner

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

  1. connect (establish connection) -> this has timeout should has timeout
  2. bind, this happen after socket connection established -> this has timeout also

OR

so what you say is that we have different thing here.

  1. connect timeout (the timeout)
  2. 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

Expand Down
1 change: 1 addition & 0 deletions jsmpp/src/main/java/org/jsmpp/session/connection/ConnectionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@
*/
public interface ConnectionFactory {
Connection createConnection(String host, int port) throws IOException;
Connection createConnection(String host, int port, long timeout) throws IOException;
}
8 changes: 8 additions & 0 deletions jsmpp/src/main/java/org/jsmpp/session/connection/socket/SocketConnectionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.jsmpp.session.connection.socket;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Socket;

import org.jsmpp.session.connection.Connection;
Expand All @@ -38,4 +39,11 @@ public Connection createConnection(String host, int port)
throws IOException {
return new SocketConnection(new Socket(host, port));
}
public Connection createConnection(String host, int port, long timeout)
throws IOException {
Socket soc = new Socket();
InetSocketAddress endpoint = new InetSocketAddress(host, port);
soc.connect(endpoint, (int)timeout);
return new SocketConnection(soc);
}
}