[4.0 -> main] P2P: Use magnitude safe time types #1321
Conversation
…'t trip heartbeat timer
[4.0] P2P: Use magnitude safe time types
| std::chrono::nanoseconds org{0}; //!< origin timestamp. Time at the client when the request departed for the server. | ||
| // std::chrono::nanoseconds (not used) rec{0}; //!< receive timestamp. Time at the server when the request arrived from the client. | ||
| std::chrono::nanoseconds xmt{0}; //!< transmit timestamp, Time at the server when the response left for the client. | ||
| // std::chrono::nanoseconds (not used) dst{0}; //!< destination timestamp, Time at the client when the reply arrived from the server. | ||
| /** @} */ | ||
| // timestamp for the lastest message | ||
| tstamp latest_msg_time{0}; | ||
| tstamp hb_timeout{std::chrono::milliseconds{def_keepalive_interval}.count()}; | ||
| tstamp latest_blk_time{0}; | ||
| std::chrono::system_clock::time_point latest_msg_time{std::chrono::system_clock::time_point::min()}; | ||
| std::chrono::milliseconds hb_timeout{std::chrono::milliseconds{def_keepalive_interval}}; | ||
| std::chrono::system_clock::time_point latest_blk_time{std::chrono::system_clock::time_point::min()}; |
There was a problem hiding this comment.
I'd prefer have these all be tstamp as before, but with tstamp defined as std::chrono::nanoseconds.
If we want to differentiate between a point in time (since the epoch), i.e time_point, and a duration, then org/rec/xmt/dst should be time_point, and not durations.
Also I don't think it is a good idea to use std::chrono::system_clock::time_point ourselves because the duration of the system_clock is not specified.
So I think the most straightforward thing might be to use tstamp defined as std::chrono::nanoseconds for both actual timestamps (i.e. time since epoch) and durations. Because it is a duration from std::chrono, we benefit from type safety insuring that we don't add std::chrono::nanoseconds and std::chrono::milliseconds, but we use a consistent internal representation. I think it would make the code simpler and clearer.
There was a problem hiding this comment.
I created #1326 as this is a merge up PR, I think it makes sense to make these changes in a separate PR.
Use
std::chronotypes instead oftstampso that magnitude is maintained. Fixes issue with heartbeat calculation being off on some machines causing pre-mature close of what it thought was a idle connection.Merges
release/4.0intomainincluding #1316Resolves #1315