Merge pull request #2866

cf5f6236 Corrections in rate limiting / trottle code, especially in 'out' direction (rbrunner7)
This commit is contained in:
Riccardo Spagni 2017-12-16 23:24:41 +02:00
commit 9b511d3e93
No known key found for this signature in database
GPG Key ID: 55432DF31CCD4FCD
8 changed files with 25 additions and 41 deletions

View File

@ -286,7 +286,7 @@ PRAGMA_WARNING_DISABLE_VS(4355)
{ {
CRITICAL_REGION_LOCAL( epee::net_utils::network_throttle_manager::network_throttle_manager::m_lock_get_global_throttle_in ); CRITICAL_REGION_LOCAL( epee::net_utils::network_throttle_manager::network_throttle_manager::m_lock_get_global_throttle_in );
epee::net_utils::network_throttle_manager::network_throttle_manager::get_global_throttle_in().handle_trafic_exact(bytes_transferred * 1024); epee::net_utils::network_throttle_manager::network_throttle_manager::get_global_throttle_in().handle_trafic_exact(bytes_transferred);
} }
double delay=0; // will be calculated - how much we should sleep to obey speed limit etc double delay=0; // will be calculated - how much we should sleep to obey speed limit etc
@ -297,7 +297,7 @@ PRAGMA_WARNING_DISABLE_VS(4355)
{ {
{ //_scope_dbg1("CRITICAL_REGION_LOCAL"); { //_scope_dbg1("CRITICAL_REGION_LOCAL");
CRITICAL_REGION_LOCAL( epee::net_utils::network_throttle_manager::m_lock_get_global_throttle_in ); CRITICAL_REGION_LOCAL( epee::net_utils::network_throttle_manager::m_lock_get_global_throttle_in );
delay = epee::net_utils::network_throttle_manager::get_global_throttle_in().get_sleep_time_after_tick( bytes_transferred ); // decission from global throttle delay = epee::net_utils::network_throttle_manager::get_global_throttle_in().get_sleep_time_after_tick( bytes_transferred );
} }
delay *= 0.5; delay *= 0.5;
@ -482,9 +482,7 @@ PRAGMA_WARNING_DISABLE_VS(4355)
//some data should be wrote to stream //some data should be wrote to stream
//request complete //request complete
if (speed_limit_is_enabled()) { // No sleeping here; sleeping is done once and for all in "handle_write"
sleep_before_packet(cb, 1, 1);
}
m_send_que_lock.lock(); // *** critical *** m_send_que_lock.lock(); // *** critical ***
epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){m_send_que_lock.unlock();}); epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){m_send_que_lock.unlock();});
@ -607,6 +605,7 @@ PRAGMA_WARNING_DISABLE_VS(4355)
} }
logger_handle_net_write(cb); logger_handle_net_write(cb);
// The single sleeping that is needed for correctly handling "out" speed throttling
if (speed_limit_is_enabled()) { if (speed_limit_is_enabled()) {
sleep_before_packet(cb, 1, 1); sleep_before_packet(cb, 1, 1);
} }

View File

@ -140,7 +140,7 @@ void cryptonote_protocol_handler_base::handler_response_blocks_now(size_t packet
{ {
CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out ); CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out );
delay = network_throttle_manager::get_global_throttle_out().get_sleep_time_after_tick( packet_size ); // decission from global delay = network_throttle_manager::get_global_throttle_out().get_sleep_time_after_tick( packet_size );
} }

View File

@ -173,14 +173,9 @@ connection_basic::~connection_basic() noexcept(false) {
} }
void connection_basic::set_rate_up_limit(uint64_t limit) { void connection_basic::set_rate_up_limit(uint64_t limit) {
// TODO remove __SCALING_FACTOR...
const double SCALING_FACTOR = 2.1; // to acheve the best performance
limit *= SCALING_FACTOR;
{ {
CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out ); CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out );
network_throttle_manager::get_global_throttle_out().set_target_speed(limit); network_throttle_manager::get_global_throttle_out().set_target_speed(limit);
network_throttle_manager::get_global_throttle_out().set_real_target_speed(limit / SCALING_FACTOR);
} }
save_limit_to_file(limit); save_limit_to_file(limit);
} }
@ -238,7 +233,7 @@ void connection_basic::sleep_before_packet(size_t packet_size, int phase, int q
{ {
CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out ); CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out );
delay = network_throttle_manager::get_global_throttle_out().get_sleep_time_after_tick( packet_size ); // decission from global delay = network_throttle_manager::get_global_throttle_out().get_sleep_time_after_tick( packet_size );
} }
delay *= 0.50; delay *= 0.50;
@ -252,7 +247,7 @@ void connection_basic::sleep_before_packet(size_t packet_size, int phase, int q
// XXX LATER XXX // XXX LATER XXX
{ {
CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out ); CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out );
network_throttle_manager::get_global_throttle_out().handle_trafic_exact( packet_size * 700); // increase counter - global network_throttle_manager::get_global_throttle_out().handle_trafic_exact( packet_size ); // increase counter - global
} }
} }
@ -262,13 +257,13 @@ void connection_basic::set_start_time() {
} }
void connection_basic::do_send_handler_write(const void* ptr , size_t cb ) { void connection_basic::do_send_handler_write(const void* ptr , size_t cb ) {
sleep_before_packet(cb,1,-1); // No sleeping here; sleeping is done once and for all in connection<t_protocol_handler>::handle_write
MTRACE("handler_write (direct) - before ASIO write, for packet="<<cb<<" B (after sleep)"); MTRACE("handler_write (direct) - before ASIO write, for packet="<<cb<<" B (after sleep)");
set_start_time(); set_start_time();
} }
void connection_basic::do_send_handler_write_from_queue( const boost::system::error_code& e, size_t cb, int q_len ) { void connection_basic::do_send_handler_write_from_queue( const boost::system::error_code& e, size_t cb, int q_len ) {
sleep_before_packet(cb,2,q_len); // No sleeping here; sleeping is done once and for all in connection<t_protocol_handler>::handle_write
MTRACE("handler_write (after write, from queue="<<q_len<<") - before ASIO write, for packet="<<cb<<" B (after sleep)"); MTRACE("handler_write (after write, from queue="<<q_len<<") - before ASIO write, for packet="<<cb<<" B (after sleep)");
set_start_time(); set_start_time();

View File

@ -72,8 +72,8 @@ namespace nodetool
{ {
namespace namespace
{ {
const int64_t default_limit_up = 2048; const int64_t default_limit_up = 2048; // kB/s
const int64_t default_limit_down = 8192; const int64_t default_limit_down = 8192; // kB/s
const command_line::arg_descriptor<std::string> arg_p2p_bind_ip = {"p2p-bind-ip", "Interface for p2p network protocol", "0.0.0.0"}; const command_line::arg_descriptor<std::string> arg_p2p_bind_ip = {"p2p-bind-ip", "Interface for p2p network protocol", "0.0.0.0"};
const command_line::arg_descriptor<std::string> arg_p2p_bind_port = { const command_line::arg_descriptor<std::string> arg_p2p_bind_port = {
"p2p-bind-port" "p2p-bind-port"
@ -1844,9 +1844,8 @@ namespace nodetool
this->islimitup=false; this->islimitup=false;
} }
limit *= 1024;
epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_up_limit( limit ); epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_up_limit( limit );
MINFO("Set limit-up to " << limit/1024 << " kB/s"); MINFO("Set limit-up to " << limit << " kB/s");
return true; return true;
} }
@ -1858,9 +1857,8 @@ namespace nodetool
limit=default_limit_down; limit=default_limit_down;
this->islimitdown=false; this->islimitdown=false;
} }
limit *= 1024;
epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_down_limit( limit ); epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_down_limit( limit );
MINFO("Set limit-down to " << limit/1024 << " kB/s"); MINFO("Set limit-down to " << limit << " kB/s");
return true; return true;
} }
@ -1872,21 +1870,21 @@ namespace nodetool
if(limit == -1) if(limit == -1)
{ {
limit_up = default_limit_up * 1024; limit_up = default_limit_up;
limit_down = default_limit_down * 1024; limit_down = default_limit_down;
} }
else else
{ {
limit_up = limit * 1024; limit_up = limit;
limit_down = limit * 1024; limit_down = limit;
} }
if(!this->islimitup) { if(!this->islimitup) {
epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_up_limit(limit_up); epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_up_limit(limit_up);
MINFO("Set limit-up to " << limit_up/1024 << " kB/s"); MINFO("Set limit-up to " << limit_up << " kB/s");
} }
if(!this->islimitdown) { if(!this->islimitdown) {
epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_down_limit(limit_down); epee::net_utils::connection<epee::levin::async_protocol_handler<p2p_connection_context> >::set_rate_down_limit(limit_down);
MINFO("Set limit-down to " << limit_down/1024 << " kB/s"); MINFO("Set limit-down to " << limit_down << " kB/s");
} }
return true; return true;

View File

@ -160,17 +160,11 @@ void network_throttle::set_target_speed( network_speed_kbps target )
{ {
m_target_speed = target * 1024; m_target_speed = target * 1024;
MINFO("Setting LIMIT: " << target << " kbps"); MINFO("Setting LIMIT: " << target << " kbps");
set_real_target_speed(target);
}
void network_throttle::set_real_target_speed( network_speed_kbps real_target )
{
m_real_target_speed = real_target * 1024;
} }
network_speed_kbps network_throttle::get_target_speed() network_speed_kbps network_throttle::get_target_speed()
{ {
return m_real_target_speed / 1024; return m_target_speed / 1024;
} }
void network_throttle::tick() void network_throttle::tick()

View File

@ -52,8 +52,7 @@ class network_throttle : public i_network_throttle {
}; };
network_speed_kbps m_target_speed; network_speed_bps m_target_speed;
network_speed_kbps m_real_target_speed;
size_t m_network_add_cost; // estimated add cost of headers size_t m_network_add_cost; // estimated add cost of headers
size_t m_network_minimal_segment; // estimated minimal cost of sending 1 byte to round up to size_t m_network_minimal_segment; // estimated minimal cost of sending 1 byte to round up to
size_t m_network_max_segment; // recommended max size of 1 TCP transmission size_t m_network_max_segment; // recommended max size of 1 TCP transmission
@ -76,7 +75,6 @@ class network_throttle : public i_network_throttle {
virtual ~network_throttle(); virtual ~network_throttle();
virtual void set_name(const std::string &name); virtual void set_name(const std::string &name);
virtual void set_target_speed( network_speed_kbps target ); virtual void set_target_speed( network_speed_kbps target );
virtual void set_real_target_speed( network_speed_kbps real_target ); // only for throttle_out
virtual network_speed_kbps get_target_speed(); virtual network_speed_kbps get_target_speed();
// add information about events: // add information about events:

View File

@ -80,7 +80,8 @@ namespace net_utils
{ {
// just typedefs to in code define the units used. TODO later it will be enforced that casts to other numericals are only explicit to avoid mistakes? use boost::chrono? // just typedefs to in code define the units used. TODO later it will be enforced that casts to other numericals are only explicit to avoid mistakes? use boost::chrono?
typedef double network_speed_kbps; typedef double network_speed_kbps; // externally, for parameters and return values, all defined in kilobytes per second
typedef double network_speed_bps; // throttle-internally, bytes per second
typedef double network_time_seconds; typedef double network_time_seconds;
typedef double network_MB; typedef double network_MB;
@ -137,7 +138,6 @@ class i_network_throttle {
public: public:
virtual void set_name(const std::string &name)=0; virtual void set_name(const std::string &name)=0;
virtual void set_target_speed( network_speed_kbps target )=0; virtual void set_target_speed( network_speed_kbps target )=0;
virtual void set_real_target_speed(network_speed_kbps real_target)=0;
virtual network_speed_kbps get_target_speed()=0; virtual network_speed_kbps get_target_speed()=0;
virtual void handle_trafic_exact(size_t packet_size) =0; // count the new traffic/packet; the size is exact considering all network costs virtual void handle_trafic_exact(size_t packet_size) =0; // count the new traffic/packet; the size is exact considering all network costs

View File

@ -1549,7 +1549,7 @@ namespace cryptonote
res.status = CORE_RPC_ERROR_CODE_WRONG_PARAM; res.status = CORE_RPC_ERROR_CODE_WRONG_PARAM;
return false; return false;
} }
epee::net_utils::connection_basic::set_rate_down_limit(nodetool::default_limit_down * 1024); epee::net_utils::connection_basic::set_rate_down_limit(nodetool::default_limit_down);
} }
if (req.limit_up > 0) if (req.limit_up > 0)
@ -1563,7 +1563,7 @@ namespace cryptonote
res.status = CORE_RPC_ERROR_CODE_WRONG_PARAM; res.status = CORE_RPC_ERROR_CODE_WRONG_PARAM;
return false; return false;
} }
epee::net_utils::connection_basic::set_rate_up_limit(nodetool::default_limit_up * 1024); epee::net_utils::connection_basic::set_rate_up_limit(nodetool::default_limit_up);
} }
res.limit_down = epee::net_utils::connection_basic::get_rate_down_limit(); res.limit_down = epee::net_utils::connection_basic::get_rate_down_limit();