Introduce SocketConnectTimeout configuration option
#638
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Stopping MINA connector can take up to a minute if there is in-flight TCP handshake which can't complete due to firewall cutting the traffic. What happens in this case is NioSocketConnector background task keeps running until connection timeout expires even after dispose() was called, and default timeout is a minute (see o.a.m.c.s.AbstractIoConnector). This behaviour is surprising: intuitively you expect closing the session to block no more than logon/logout timeout; but currently even with stricter than default settings it's still not safe to do this from a time-sensitive thread. This patch makes connection timeout configurable on QuickFIX side - which may be a bit of a workaround, but seems like a generally useful socket option. Default value for now matches MINA for backward compatibility. However, it can potentially be lowered to something like 5 seconds (half of the default LogonTimeout), and then reconnection logic would still ensure the session is eventually established.
|
Hi @AndreyNudko , thanks for the PR. This is definitely useful. |
chrjohn
changed the title
SocketConnectTimeout configuration option
chrjohn
reviewed
Dec 8, 2023
quickfixj-core/src/main/java/quickfix/mina/initiator/AbstractSocketInitiator.java
Outdated
Show resolved
Hide resolved
chrjohn
requested changes
Dec 11, 2023
quickfixj-core/src/main/java/quickfix/mina/initiator/AbstractSocketInitiator.java
Show resolved
Hide resolved
chrjohn
approved these changes
Jun 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stopping MINA connector can take up to a minute if there is in-flight TCP handshake which can't complete due to firewall cutting the traffic. What happens in this case is NioSocketConnector background task keeps running until connection timeout expires even after dispose() was called, and default timeout is a minute (see o.a.m.c.s.AbstractIoConnector). This behaviour is surprising: intuitively you expect closing the session to block no more than logon/logout timeout; but currently even with stricter than default settings it's still not safe to do this from a time-sensitive thread.
This patch makes connection timeout configurable on QuickFIX side - which may be a bit of a workaround, but seems like a generally useful socket option. Default value for now matches MINA for backward compatibility. However, it can potentially be lowered to something like 5 seconds (half of the default LogonTimeout), and then reconnection logic would still ensure the session is eventually established.