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

TCP Client Connection Factory Improvements #2866

Open
garyrussell opened this issue Mar 28, 2019 · 7 comments
Open

TCP Client Connection Factory Improvements #2866

garyrussell opened this issue Mar 28, 2019 · 7 comments

Comments

@garyrussell
Copy link
Contributor

Add getConnection(String connectionId) so the client can select which connection it wants (if not open, return null).

Also singleUse serves 2 purposes when true

  • tells the CF to hand out a new connection each time getConnection() is called
  • tells the adapters to close the socket when the send/receive is complete (one-way) or after the reply is sent/received (request/reply).

Split this into two flags sharedConnection and closeAfterCompletion.

See https://stackoverflow.com/questions/55404438/programmatically-create-multiple-connections-for-tcpnetclientconnectionfactory/55404817#55404817

@garyrussell garyrussell added this to the 5.2.x milestone Mar 28, 2019
@garyrussell garyrussell self-assigned this Mar 28, 2019
@garyrussell garyrussell modified the milestones: 5.3.x, 5.3 RC1, Backlog Mar 31, 2020
@deepak5127
Copy link
Contributor

Hi @garyrussell , Shall i work on this item ?

@artembilan
Copy link
Member

@deepak5127 ,

sure! You can take a crack on this and we together can see what to do else in your Pull Request!

@deepak5127
Copy link
Contributor

@artembilan I have couple of questions.

  1. Should i replace singleUse boolean with two boolean flags? Like replace singleUse in AbstractConnectionFactory with two booleans mentioned above ?
  2. If so it have changes in both client and server factory classes. Also if i understand correctly, we need to use sharedConnection in connection factory for creating / re-using the connections and closeAfterCompletion in adapters to close/keep the connections ?
  3. Also new method getConnection(String connectionId) , should we implement for ThreadAffinityClientConnectionFactory and CachingClientConnectionFactory alone ?

@garyrussell
Copy link
Contributor Author

  1. Yes; but we need to keep (and deprecate) the getter and setter for the old flag (and set both).
  2. Correct.
  3. No; it should be implemented by all client factories; the adapters will need a mechanism to decide which connection id to obtain.

This is a non-trivial change (hence it's been on the backlog for a while).

@deepak5127
Copy link
Contributor

Thanks @garyrussell . Just to make sure I understand correctly. Instead of calling "this.clientConnectionFactory.getConnection() " from adapters, now we should extract the ip_connectionId from message and call this.clientConnectionFactory.getConnection(connectionId).

If the connection is already created , then send that back or create a new one. Also we need to create a Map in AbstractClientConnectionFactory to hold multiple connections, since we dont have that.

@garyrussell
Copy link
Contributor Author

No; it should be optional only; by default use the existing getConnection().

Add a SpEL expression property; something like connectionIdExpression; which, if present, tells us to try to get the existing connection by name and, if not present, fall back to the existing getConnection() - we might also need. to provide an option to throw an exception if the connection doesn't exist already.

In most cases, yes, the expression will be headers[ip_connectionId].

Yes, we'll need a map of connections.

@deepak5127
Copy link
Contributor

Thanks @garyrussell . I will work on it.

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

3 participants