Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Socket timeout tidyup #3704

Merged
merged 10 commits into from Feb 3, 2022
Merged

Conversation

dsiganos
Copy link
Contributor

@dsiganos dsiganos commented Feb 2, 2022

Tidy up of how timeouts are handled in socket class.

  • Use guard ifs to make nano::socket::async_write more readable
  • Removed needless code
  • Added comments to nano::socket class
  • Renamed the nano::socket concept of deadline to timeout
    A deadline makes people imagine a fixed point in time, a timestamp.
    However, socket::next_deadline is a timeout value and not a deadline.
    It signifies the seconds of inactivity that would cause a socket timeout.
  • Remove network constant socket_dev_idle_timeout
  • Rename socket::stop_timer to socket:set_last_completion
    All stop timer does is to set a variable. It is a simply setter function.
    It is confusing to call it stop_timer because it implies that it does
    something more than just set a timestamp.
  • Added some socket_timeout unit tests

All stop timer does is to set a variable. It is a simply setter function.
It is confusing to call it stop_timer because it implies that it does
something more than just set a timestamp.
To make it obvious it is setter function and to make it consistent with
other setter functions.
A deadline makes people imagine a fixed point in time, a timestamp.
However, socket::next_deadline is a timeout value and not a deadline.
It signifies the seconds of inactivity that would cause a socket timeout.

Renames:
set_next_deadline          -> set_default_timeout
set_next_deadline(timeout) -> set_timeout(timeout)
io_timeout                 -> default_timeout
timeout_set                -> set_default_timeout_value
next_deadline              -> timeout
@thsfs
Copy link
Contributor

thsfs commented Feb 2, 2022

It seems there is no major change other than tidying it up. They seem to be ok and the comments will be very helpful. @dsiganos, if you want to remove one more thing I could point the function socket::set_silent_connection_tolerance_time (std::chrono::seconds tolerance_time_a);. I added it to facilitate injecting a tolerance time for testing, also because it'd rather read if from the network_params, but it ended up unused.

@zhyatt zhyatt added the quality improvements This item indicates the need for or supplies changes that improve maintainability label Feb 3, 2022
@zhyatt zhyatt added this to the V24.0 milestone Feb 3, 2022
@dsiganos dsiganos merged commit 275a545 into nanocurrency:develop Feb 3, 2022
@dsiganos dsiganos deleted the socket-timeout-tidyup branch February 3, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality improvements This item indicates the need for or supplies changes that improve maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants