From 2abe42741743bd8eeaf37350acb20b4a10b7901e Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Mon, 16 May 2016 14:12:13 -0700 Subject: [PATCH] Revert of Increase the stun ping interval. (patchset #5 id:80001 of https://codereview.webrtc.org/1944003002/ ) Reason for revert: This will take longer time for the RTT to converge. Need to update the RTT calculation algorithm if doing this. Original issue's description: > Increase the stun ping interval. > > Writable connections are pinged at a slower rate. > The function IsPingable will filter out the writable connections. > The interval for slower ping rate by default is WRITABLE_CONNECTION_PING_INTERVAL(2500ms) and can be set with the configuration. > > BUG=webrtc:1161 > > Committed: https://crrev.com/8f7a5aad55a64f0d81b6436a22ffbdfcdcde91e0 > Cr-Commit-Position: refs/heads/master@{#12736} TBR=honghaiz@webrtc.org,pthatcher@webrtc.org,zhihuang@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:1161 Review URL: https://codereview.webrtc.org/1982713002 . Cr-Commit-Position: refs/heads/master@{#12762} --- webrtc/p2p/base/p2ptransportchannel.cc | 31 +++++++------------ .../p2p/base/p2ptransportchannel_unittest.cc | 5 ++- webrtc/p2p/base/transport.h | 11 ++++--- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 5e60b278c1..2801c44646 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -217,8 +217,10 @@ static const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 1000; // writable or not receiving. const int WEAK_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 10000; -// Writable connections are pinged at a slower rate. -static const int WRITABLE_CONNECTION_PING_INTERVAL = 2500; // ms +// If the current best connection is both writable and receiving, then we will +// also try hard to make sure it is pinged at this rate (a little less than +// 2 * STRONG_PING_INTERVAL). +static const int MAX_CURRENT_STRONG_INTERVAL = 900; // ms static const int MIN_CHECK_RECEIVING_INTERVAL = 50; // ms @@ -248,7 +250,7 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, 0 /* backup_connection_ping_interval */, false /* gather_continually */, false /* prioritize_most_likely_candidate_pairs */, - WRITABLE_CONNECTION_PING_INTERVAL) { + MAX_CURRENT_STRONG_INTERVAL /* max_strong_interval */) { uint32_t weak_ping_interval = ::strtoul( webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(), nullptr, 10); @@ -431,13 +433,11 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { LOG(LS_INFO) << "Set ping most likely connection to " << config_.prioritize_most_likely_candidate_pairs; - if (config.writable_connection_ping_interval >= 0 && - config_.writable_connection_ping_interval != - config.writable_connection_ping_interval) { - config_.writable_connection_ping_interval = - config.writable_connection_ping_interval; - LOG(LS_INFO) << "Set writable_connection_ping_interval to " - << config_.writable_connection_ping_interval; + if (config.max_strong_interval >= 0 && + config_.max_strong_interval != config.max_strong_interval) { + config_.max_strong_interval = config.max_strong_interval; + LOG(LS_INFO) << "Set max strong interval to " + << config_.max_strong_interval; } } @@ -1307,7 +1307,6 @@ void P2PTransportChannel::OnCheckAndPing() { MarkConnectionPinged(conn); } } - int delay = std::min(ping_interval, check_receiving_interval_); thread()->PostDelayed(delay, this, MSG_CHECK_AND_PING); } @@ -1349,13 +1348,6 @@ bool P2PTransportChannel::IsPingable(Connection* conn, int64_t now) { return (now >= conn->last_ping_response_received() + config_.backup_connection_ping_interval); } - - // Writable connections are pinged at a slower rate. - if (conn->writable()) { - return (now >= - conn->last_ping_sent() + config_.writable_connection_ping_interval); - } - return conn->active(); } @@ -1371,8 +1363,7 @@ Connection* P2PTransportChannel::FindNextPingableConnection() { Connection* conn_to_ping = nullptr; if (best_connection_ && best_connection_->connected() && best_connection_->writable() && - (best_connection_->last_ping_sent() + - config_.writable_connection_ping_interval <= + (best_connection_->last_ping_sent() + config_.max_strong_interval <= now)) { conn_to_ping = best_connection_; } else { diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 51933d4a9e..54ab3196cb 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -2696,14 +2696,13 @@ class P2PTransportChannelMostLikelyToWorkFirstTest cricket::P2PTransportChannel& StartTransportChannel( bool prioritize_most_likely_to_work, - int writable_connection_ping_interval) { + int max_strong_interval) { channel_.reset( new cricket::P2PTransportChannel("checks", 1, nullptr, allocator())); cricket::IceConfig config = channel_->config(); config.prioritize_most_likely_candidate_pairs = prioritize_most_likely_to_work; - config.writable_connection_ping_interval = - writable_connection_ping_interval; + config.max_strong_interval = max_strong_interval; channel_->SetIceConfig(config); PrepareChannel(channel_.get()); channel_->Connect(); diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index 68f178a40a..e31d37a6f6 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -154,22 +154,23 @@ struct IceConfig { // is writable yet. bool prioritize_most_likely_candidate_pairs = false; - // Writable connections are pinged at a slower rate. - int writable_connection_ping_interval = -1; + // If the current best connection is both writable and receiving, + // then we will also try hard to make sure it is pinged at this rate + // (Default value is a little less than 2 * STRONG_PING_INTERVAL). + int max_strong_interval = -1; IceConfig() {} IceConfig(int receiving_timeout_ms, int backup_connection_ping_interval, bool gather_continually, bool prioritize_most_likely_candidate_pairs, - int writable_connection_ping_interval_ms) + int max_strong_interval_ms) : receiving_timeout(receiving_timeout_ms), backup_connection_ping_interval(backup_connection_ping_interval), gather_continually(gather_continually), prioritize_most_likely_candidate_pairs( prioritize_most_likely_candidate_pairs), - writable_connection_ping_interval( - writable_connection_ping_interval_ms) {} + max_strong_interval(max_strong_interval_ms) {} }; bool BadTransportDescription(const std::string& desc, std::string* err_desc);