Skip to content

Add a timeout for the socket connection#78

Merged
shroffk merged 5 commits into
epics-base:masterfrom
rjwills28:socket_connect_timeout
Mar 13, 2024
Merged

Add a timeout for the socket connection#78
shroffk merged 5 commits into
epics-base:masterfrom
rjwills28:socket_connect_timeout

Conversation

@rjwills28

Copy link
Copy Markdown
Contributor

This PR addresses the issue described in #36 where the method to connect to a socket can block for ~ 2 minutes. Instead, I have added a timeout for the connection, which is configurable in the properties. A default of 5 seconds has been used for the timeout if not specified.

@kasemir

kasemir commented Feb 1, 2024

Copy link
Copy Markdown
Contributor

Looks good to me: No change unless one reduces the now-configurable connection timeout

@rjwills28

Copy link
Copy Markdown
Contributor Author

@kasemir thanks for taking a look at this. I set the configurable connection timeout back to 2 minutes to resemble the original behaviour of the open() method (which took ~2 mins to timeout). If you don't think this is necessary then I can reduce it?

@shroffk shroffk merged commit 669385d into epics-base:master Mar 13, 2024
@kasemir

kasemir commented Jun 14, 2024

Copy link
Copy Markdown
Contributor

I set the configurable connection timeout back to 2 minutes

I'd say the default can be shorter.
Of course unclear what a good default is.
Should the TCP connection succeed in ~1 second?
I think the command line "caget" has a 1 second default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants