[ZOOKEEPER-3384] Add SO_LINGER socket option to avoid long quorum unavailable during close with full send buffer #1085
Conversation
…lose with full send buffer
eolivelli
left a comment
eolivelli
left a comment
There was a problem hiding this comment.
Lgtm
Just for curiosity, did you test with jdk12/jdk13? Or are you using jdk8?
The behavior of SO_LINGER + socket#close changed a bit.
|
@eolivelli our test and prod environement are running on JDK9 and JDK10, haven't tested on JDK beyond that. Can you point me the SO_LINGER behavior change on JDK 12/13? |
|
retest this please |
anmolnar
left a comment
anmolnar
left a comment
There was a problem hiding this comment.
Please add documentation and possibly a simple unit test.
|
|
||
| public static void setSocketOption(Socket sock, int soTimeout) throws SocketException { | ||
| sock.setSoTimeout(soTimeout); | ||
| sock.setSoLinger(true, soLinger); |
There was a problem hiding this comment.
There's no way to turn it off. Is that intentional?
There was a problem hiding this comment.
It seems soLinger is always useful, and set the value to a big value is kind of turning it off, that's why we didn't add option to set it to false. Do you suggest to add it?
|
@anmolnar adding the test for SoLinger behavior would be tricky, and that's kind of testing the TCP socket behavior with this setting, which seems not necessary, were you suggesting if we only add a get/set test for SoLinger? |
|
IMHO there is no strict need of a test in this case |
|
@anmolnar based on the comments, do you think we still need to add a test for this? |
|
retest maven build |
|
@lvfangmin Nope, I think we're good to go now, but please add short documentation to the admin docs and I'll merge. |
|
We found the SO_LINGER option is not honored in JDK 11+, so we're changing the code to close the quorum connection in async way. Jie is upstreaming that change in ZOOKEEPER-3574: Close quorum socket asynchronously and ZOOKEEPER-3575: Moving sending packets in Learner to a separate thread, I'll abandon this one. |
3 participants
No description provided.