Alternative fix for issue 4014, where we always send pings on ROUTER connections #4016
Conversation
server/client.go
Outdated
| } else if delta := now.Sub(c.ping.last); delta < pingInterval && !needRTT { | ||
| // If we received a ping from the other side within the PingInterval then | ||
| // there is no need to send a ping. | ||
| if delta := now.Sub(c.ping.last); delta < pingInterval && !needRTT { |
There was a problem hiding this comment.
This feels like it should be any inbound activity not just a ping.
There was a problem hiding this comment.
So other inbound data from a client connection will not suppress the pings here, that is the intended behavior correct?
There was a problem hiding this comment.
Well no. As per your suggestion on the other PR, in this update the c.ping.last timestamp is used to record both received pings and client data. There is another change to the end of the readLoop routine where c.ping.last is updated.
There was a problem hiding this comment.
ok maybe rename then to in, so c.in.last or c.inbound.last?
There was a problem hiding this comment.
I think we are close, and we are prepping to release 2.9.16.
If we can get this merged later today or no later then tomorrow it will make it in.
There was a problem hiding this comment.
OK. Not 100% sure this is what you meant, but I have changed the ping check to work with a c.lastIn timestamp, which is updated on receiving client data or a ping, and deleted the c.ping.last field.
sandykellagher
force-pushed
the
Issue_4014_always_ping_ROUTER_connections
branch
from
41ff5b9
to
5c5f773
Compare
|
I've made a change so that the c.ping.last timestamp is updated for receive client data as well as pings, and squashed this commit with the previous one. I've also marked the other PR as closed. |
…connections, updating c.lastIn timestamp on receiving client data or ping
sandykellagher
force-pushed
the
Issue_4014_always_ping_ROUTER_connections
branch
from
d7b2ab5
to
5ae83b7
Compare
Alternative fix for issue #4014: where we always send pings for ROUTER, GATEWAY and LEAF spoke connections.
This is my own original work that I license to the project
[ x] Link to issue, e.g. Resolves #NNN
Documentation added (if applicable)
Tests added
[ x] Branch rebased on top of current main (git pull --rebase origin main)
[ x] Changes squashed to a single commit (described here)
[x] Build is green in Travis CI
[ x] You have certified that the contribution is your original work and that you license the work to the project under the Apache 2 license
Resolves #
Changes proposed in this pull request: