FEATURE: Add TCP connection keep-alive option. #673
Merged
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
jhpark816
reviewed
Sep 13, 2023
| @@ -919,6 +922,9 @@ private void handleIO(SelectionKey sk) { | |||
| if (sk.isConnectable()) { | |||
| getLogger().info("Connection state changed for %s", qa); | |||
| final SocketChannel channel = qa.getChannel(); | |||
| Socket socket = channel.socket(); | |||
| socket.setTcpNoDelay(!connFactory.useNagleAlgorithm()); | |||
| socket.setKeepAlive(keepAlive); | |||
There was a problem hiding this comment.
connFactory.getKeepAlive()사용
connFactory.getKeepAlive() 값이 true인 경우만 socket,setKeepAlive() 호출하는 것보다, true/false 관계없이 무조건 호출하는 것이 나은 가요?
There was a problem hiding this comment.
ConnectionFactoryBuilder 내의 arcusMigrEnabled, arcusMigrEnabled 등의 다른 property 변수와의 일관성을 위해 위와 같이 구현하였습니다.
There was a problem hiding this comment.
tcp socket option을 설정하는 useNagleAlgorithm()와 일관성을 맞춰주세요.
brido4125
force-pushed
the
feature/keepAlive
branch
from
43a47cd
to
bfa7f2a
Compare
brido4125
force-pushed
the
feature/keepAlive
branch
2 times, most recently
from
7784389
to
3ae1795
Compare
uhm0311
reviewed
Sep 14, 2023
| @@ -59,6 +59,7 @@ public class ConnectionFactoryBuilder { | |||
| private boolean isDaemon = true; | |||
| private boolean shouldOptimize = false; | |||
| private boolean useNagle = false; | |||
| protected boolean keepAlive = false; | |||
brido4125
force-pushed
the
feature/keepAlive
branch
from
3ae1795
to
6d418fa
Compare
uhm0311
reviewed
Sep 14, 2023
| @Override | ||
| public boolean getKeepAlive() { | ||
| return inner.getKeepAlive(); | ||
| } |
There was a problem hiding this comment.
ConnectionFactoryBuilderTest 클래스에 keepAlive 관련 테스트를 추가합시다.
brido4125
force-pushed
the
feature/keepAlive
branch
from
6d418fa
to
8ac00dd
Compare
uhm0311
approved these changes
Sep 14, 2023
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.
관련 이슈
https://github.com/jam2in/arcus-works/issues/441
Motivation
기존 구현은 사용자가 TCP Keep Alive 옵션을 설정하는 인터페이스를 제공하지 않는다.
이를
ConnectionFactoryBuilder를 통해 사용자가 설정할 수 있도록 제공한다.keepAlive의 기본 옵션은
false이다.