Change SocketCache to be shared between nodes #2501
Conversation
ganmacs
changed the title
ganmacs
changed the title
ganmacs
force-pushed
the
socket-cache-udpate
branch
from
c684e2e
to
972638b
Compare
ganmacs
force-pushed
the
socket-cache-udpate
branch
4 times, most recently
from
9a77fa9
to
407f963
Compare
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
ganmacs
force-pushed
the
socket-cache-udpate
branch
from
407f963
to
6c604fc
Compare
ganmacs
changed the title
| # This method is thread unsafe | ||
| def expired?(key = Thread.current.object_id) | ||
| @active_socks[key].timeout ? @active_socks[key].timeout < Time.now : false | ||
| def expired_socket?(sock, time: Time.now) |
There was a problem hiding this comment.
This method is private so normal argument seems enough for time. Not important comment.
| end | ||
| end | ||
|
|
||
| def clear | ||
| def checkin(sock) |
There was a problem hiding this comment.
Hmm... checkin is unclear for me. After checkin, we are inflight for airplane but this method changes inflight to available. checkout too. Maybe, we don't use checkout for flight.
There was a problem hiding this comment.
I referred connection_pool gem to determine the name of them.
https://github.com/mperham/connection_pool/blob/99b81aa0a9dccb3b423260740ce2d603f1479356/lib/connection_pool.rb#L80
Any name is okay for me if it's clear for readers. Do you have any suggestions?
There was a problem hiding this comment.
Hmm.... I see. If this naming is popular in ruby, it is okay for me.
| @inactive_sockets.clear | ||
| end | ||
|
|
||
| while (s = sockets.pop) |
There was a problem hiding this comment.
each is more lightweight due to no sockets update.
| else | ||
| @log.warn("Not found key for dec_ref: #{key}") | ||
| end | ||
| while (s = sockets.pop) |
| @log.debug("connect new socket #{new_tsock}") | ||
|
|
||
| @inflight_sockets[val] = new_tsock | ||
| return new_tsock.sock |
There was a problem hiding this comment.
To reduce return by if
if tsock
tsock.sock
else
# same
end
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
| if tsock | ||
| tsock.sock | ||
| else | ||
| val = yield |
There was a problem hiding this comment.
sock or new_sock is better than val
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
|
looks good. |
Which issue(s) this PR fixes:
no
What this PR does / why we need it:
Old SocketCache isn't shared between other nodes since node's
portandhostis fixed at start time.In the situation nodes' port and host change dynamically, sharing SocketCache between nodes makes more efficient.
And recent refactoring can make SocketCache more simple.
Docs Changes:
no need
Release Note:
no need