Ping the premier connection on each network with higher priority.

webrtc/p2p/base/p2ptransportchannel.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 1230 matching lines @@
   }
 
   Connection* top_connection =
       (connections_.size() > 0) ? connections_[0] : nullptr;
 
   // If necessary, switch to the new choice. Note that |top_connection| doesn't
   // have to be writable to become the selected connection although it will
   // have higher priority if it is writable.
   MaybeSwitchSelectedConnection(top_connection, "sorting");
 
+  UpdatePremierConnections();
+
   // The controlled side can prune only if the selected connection has been
   // nominated because otherwise it may prune the connection that will be
   // selected by the controlling side.
   // TODO(honghaiz): This is not enough to prevent a connection from being
   // pruned too early because with aggressive nomination, the controlling side
   // will nominate every connection until it becomes writable.
   if (ice_role_ == ICEROLE_CONTROLLING ||
       (selected_connection_ && selected_connection_->nominated())) {
     PruneConnections();
   }
@@ skipping 17 matching lines @@
   UpdateState();
 
   // Also possibly start pinging.
   // We could start pinging if:
   // * The first connection was created.
   // * ICE credentials were provided.
   // * A TCP connection became connected.
   MaybeStartPinging();
 }
 
+void P2PTransportChannel::UpdatePremierConnections() {
+  // |connections_| has been sorted, so the first one in the list on a given
+  // network is the premier connection, except that the selected connection
+  // is always the premier connection on the network.
+  premier_connection_by_network_name_.clear();
+  for (Connection* conn : connections_) {
+    const std::string& network_name = conn->port()->Network()->name();
+    if (conn == selected_connection_ ||
+        premier_connection_by_network_name_.find(network_name) ==
+            premier_connection_by_network_name_.end()) {
+      premier_connection_by_network_name_[network_name] = conn;
+    }
+  }
+}
+
 void P2PTransportChannel::PruneConnections() {
   // We can prune any connection for which there is a connected, writable
   // connection on the same network with better or equal priority.  We leave
   // those with better priority just in case they become writable later (at
   // which point, we would prune out the current selected connection).  We leave
   // connections on other networks because they may not be using the same
   // resources and they may represent very distinct paths over which we can
   // switch. If the |premier| connection is not connected, we may be
   // reconnecting a TCP connection and temporarily do not prune connections in
   // this network. See the big comment in CompareConnectionStates.
-
-  // Get a list of the networks that we are using.
-  std::set<rtc::Network*> networks;
-  for (const Connection* conn : connections_) {
-    networks.insert(conn->port()->Network());
-  }
-  for (rtc::Network* network : networks) {
-    Connection* premier = GetBestConnectionOnNetwork(network);
-    // Do not prune connections if the current selected connection is weak on
+  for (Connection* conn : connections_) {
+    // Do not prune connections if the current premier connection is weak on
     // this network. Otherwise, it may delete connections prematurely.
-    if (!premier || premier->weak()) {
+    Connection* premier =
+        premier_connection_by_network_name_[conn->port()->Network()->name()];
+    if (!premier || conn == premier || premier->weak()) {
       continue;
     }
-
-    for (Connection* conn : connections_) {
-      if ((conn != premier) && (conn->port()->Network() == network) &&
-          (CompareConnectionCandidates(premier, conn) >= 0)) {
-        conn->Prune();
-      }
+    if (CompareConnectionCandidates(premier, conn) >= 0) {
+      conn->Prune();
     }
   }
 }
 
 // Change the selected connection, and let listeners know.
 void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) {
   // Note: if conn is NULL, the previous |selected_connection_| has been
   // destroyed, so don't use it.
   Connection* old_selected_connection = selected_connection_;
   selected_connection_ = conn;
@@ skipping 117 matching lines @@
 bool P2PTransportChannel::ReadyToSend(Connection* connection) const {
   // Note that we allow sending on an unreliable connection, because it's
   // possible that it became unreliable simply due to bad chance.
   // So this shouldn't prevent attempting to send media.
   return connection != nullptr &&
          (connection->writable() ||
           connection->write_state() == Connection::STATE_WRITE_UNRELIABLE ||
           PresumedWritable(connection));
 }
 
-// If we have a selected connection, return it, otherwise return top one in the
-// list (later we will mark it best).
-Connection* P2PTransportChannel::GetBestConnectionOnNetwork(
-    rtc::Network* network) const {
-  // If the selected connection is on this network, then it wins.
-  if (selected_connection_ &&
-      (selected_connection_->port()->Network() == network)) {
-    return selected_connection_;
-  }
-
-  // Otherwise, we return the top-most in sorted order.
-  for (size_t i = 0; i < connections_.size(); ++i) {
-    if (connections_[i]->port()->Network() == network) {
-      return connections_[i];
-    }
-  }
-
-  return NULL;
-}
-
 // Handle any queued up requests
 void P2PTransportChannel::OnMessage(rtc::Message *pmsg) {
   switch (pmsg->message_id) {
     case MSG_SORT_AND_UPDATE_STATE:
       SortConnectionsAndUpdateState();
       break;
     case MSG_CHECK_AND_PING:
       OnCheckAndPing();
       break;
     case MSG_REGATHER_ON_FAILED_NETWORKS:
@@ skipping 57 matching lines @@
     return false;
   }
 
   // An never connected connection cannot be written to at all, so pinging is
   // out of the question. However, if it has become WRITABLE, it is in the
   // reconnecting state so ping is needed.
   if (!conn->connected() && !conn->writable()) {
     return false;
   }
 
-  // If the channel is weakly connected, ping all connections.
-  if (weak()) {
-    return true;
-  }
-
-  // Always ping active connections regardless whether the channel is completed
-  // or not, but backup connections are pinged at a slower rate.
-  if (IsBackupConnection(conn)) {
+  // Backup connections are pinged at a slower rate if the channel is strongly
+  // connected.
+  bool strongly_connected = !weak();
+  if (strongly_connected && IsBackupConnection(conn)) {
     return conn->rtt_samples() == 0 ||
            (now >= conn->last_ping_response_received() +
                        config_.backup_connection_ping_interval);
   }
-  // Don't ping inactive non-backup connections.
+
+  // If the premier connection is weakly connected, ping all connections on

[Taylor Brandstetter, 2016/10/04 02:08:10]

Suggest changing this to "If the premier connection on a network is weakly
connected..." to make it clear there are multiple premier connections.

[honghaiz3, 2016/10/20 23:47:58]

Done.

+  // the same network. Otherwise, do not ping those with a lower priority
+  // (i.e., those having been pruned) or those which have timed out.
+  auto iter =
+      premier_connection_by_network_name_.find(conn->port()->Network()->name());
+  if (iter == premier_connection_by_network_name_.end() || !iter->second ||
+      iter->second->weak()) {
+    return true;
+  }
+
+  // Don't ping inactive connections.
   if (!conn->active()) {
     return false;
   }
 
   // Do ping unwritable, active connections.
   if (!conn->writable()) {
     return true;
   }
 
   // Ping writable, active connections if it's been long enough since the last
   // ping.
   int ping_interval = CalculateActiveWritablePingInterval(conn, now);
   return (now >= conn->last_ping_sent() + ping_interval);
 }
 
-bool P2PTransportChannel::IsSelectedConnectionPingable(int64_t now) {
-  if (!selected_connection_ || !selected_connection_->connected() ||
-      !selected_connection_->writable()) {
+bool P2PTransportChannel::IsPremierConnectionPingable(
+    Connection* premier_connection,
+    int64_t now) {
+  if (!premier_connection || !premier_connection->connected() ||
+      !premier_connection->writable() ||
+      premier_connection->state() == Connection::STATE_FAILED) {

[Taylor Brandstetter, 2016/10/04 02:08:09]

Why did the "failed" check not exist before?

[honghaiz3, 2016/10/20 23:47:58]

It is logical to include this although it does not matter here
because whenever we set a connection to STATE_FAILED, we also mark it as
WRITE_TIMEOUT. Revert this change to avoid the confusion. 

     return false;
   }
 
-  int interval = CalculateActiveWritablePingInterval(selected_connection_, now);
-  return selected_connection_->last_ping_sent() + interval <= now;
+  int interval = CalculateActiveWritablePingInterval(premier_connection, now);
+  return premier_connection->last_ping_sent() + interval <= now;
+}
+
+Connection* P2PTransportChannel::FindBestPremierConnectionToPing(int64_t now) {
+  // If the selected connection is pingable, select it to ping.
+  if (IsPremierConnectionPingable(selected_connection_, now)) {
+    return selected_connection_;
+  }
+
+  // Otherwise, find the premier connection that has not been pinged for the
+  // longest time.
+  Connection* oldest_premier_connection = nullptr;
+  for (auto kv : premier_connection_by_network_name_) {
+    Connection* conn = kv.second;
+    if (conn == nullptr || conn == selected_connection_) {
+      continue;
+    }
+    if (IsPremierConnectionPingable(conn, now) &&
+        (!oldest_premier_connection ||
+         conn->last_ping_sent() <
+             oldest_premier_connection->last_ping_sent())) {
+      oldest_premier_connection = conn;
+    }
+  }
+  return oldest_premier_connection;
 }
 
 int P2PTransportChannel::CalculateActiveWritablePingInterval(
     const Connection* conn,
     int64_t now) const {
   // Ping each connection at a higher rate at least
   // MIN_PINGS_AT_WEAK_PING_INTERVAL times.
   if (conn->num_pings_sent() < MIN_PINGS_AT_WEAK_PING_INTERVAL) {
     return weak_ping_interval_;
   }
 
   int stable_interval = config_.stable_writable_connection_ping_interval;
   int stablizing_interval =
       std::min(stable_interval, STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL);
-
-  return conn->stable(now) ? stable_interval : stablizing_interval;
+  // If the channel is weak or the connection is not stable yet, use the
+  // stablizing_interval.
+  return (!weak() && conn->stable(now)) ? stable_interval : stablizing_interval;

[Taylor Brandstetter, 2016/10/04 02:08:09]

So this is now the "stabilizing or weak" interval? I think that needs to be made
clear. Maybe a new interval should be introduced.

[honghaiz3, 2016/10/20 23:47:58]

Changed to weak_or_stablizing_interval. 

 }
 
 // Returns the next pingable connection to ping.  This will be the oldest
 // pingable connection unless we have a connected, writable connection that is
 // past the writable ping interval. When reconnecting a TCP
 // connection, the selected connection is disconnected, although still WRITABLE
 // while reconnecting. The newly created connection should be selected as the
 // ping target to become writable instead. See the big comment in
 // CompareConnectionStates.
 Connection* P2PTransportChannel::FindNextPingableConnection() {
   int64_t now = rtc::TimeMillis();
-  Connection* conn_to_ping = nullptr;
-  if (IsSelectedConnectionPingable(now)) {
-    conn_to_ping = selected_connection_;
-  } else {
+  Connection* conn_to_ping = FindBestPremierConnectionToPing(now);
+  if (!conn_to_ping) {
     conn_to_ping = FindConnectionToPing(now);
   }
   return conn_to_ping;
 }
 
 void P2PTransportChannel::MarkConnectionPinged(Connection* conn) {
   if (conn && pinged_connections_.insert(conn).second) {
     unpinged_connections_.erase(conn);
   }
 }
@@ skipping 93 matching lines @@
   // Note: the previous selected_connection_ may be destroyed by now, so don't
   // use it.
 
   // Remove this connection from the list.
   std::vector<Connection*>::iterator iter =
       std::find(connections_.begin(), connections_.end(), connection);
   ASSERT(iter != connections_.end());
   pinged_connections_.erase(*iter);
   unpinged_connections_.erase(*iter);
   connections_.erase(iter);
+  for (auto kv : premier_connection_by_network_name_) {
+    if (kv.second == connection) {
+      kv.second = nullptr;
+    }
+  }
 
-  LOG_J(LS_INFO, this) << "Removed connection ("
-    << static_cast<int>(connections_.size()) << " remaining)";
+  LOG_J(LS_INFO, this) << "Removed connection " << std::hex << connection

[Taylor Brandstetter, 2016/10/04 02:08:10]

Why not log connection->ToString()?

[honghaiz3, 2016/10/20 23:47:58]

connection->ToString() needs to access the members in the respective port. There
is potential risk that the port may also be in the process of deleting itself
and the members are partially released. 
Plus, it is probably sufficient to know which connection is removed at this
point.

[Taylor Brandstetter, 2016/10/21 20:18:49]

Does that risk really exist? I thought a Port is only destroyed asynchronously,
after all its connections are destroyed.

[honghaiz3, 2016/10/21 21:27:26]

It probably does not although it would require careful analysis of the code. 
I once got caught when I tried to print out the whole information of a
Connection when it is about to be deleted and it caused some tests flaky after
the CL was landed. 
It is not super important to print out the complete information here.  
It is probably sufficient to know which connection was deleted at this point for
the purpose of debugging.  

+                       << std::dec << " ("
+                       << static_cast<int>(connections_.size())
+                       << " remaining)";
 
   // If this is currently the selected connection, then we need to pick a new
   // one. The call to SortConnectionsAndUpdateState will pick a new one. It
   // looks at the current selected connection in order to avoid switching
   // between fairly similar ones. Since this connection is no longer an option,
   // we can just set selected to nullptr and re-choose a best assuming that
   // there was no selected connection.
   if (selected_connection_ == connection) {
     LOG(LS_INFO) << "Selected connection destroyed. Will choose a new one.";
     SwitchSelectedConnection(nullptr);
@@ skipping 107 matching lines @@
 }
 
 void P2PTransportChannel::OnReadyToSend(Connection* connection) {
   if (connection == selected_connection_ && writable()) {
     SignalReadyToSend(this);
   }
 }
 
 // Find "triggered checks".  We ping first those connections that have
 // received a ping but have not sent a ping since receiving it
-// (last_received_ping > last_sent_ping).  But we shouldn't do
+// (last_ping_received > last_ping_sent).  But we shouldn't do
 // triggered checks if the connection is already writable.
 Connection* P2PTransportChannel::FindOldestConnectionNeedingTriggeredCheck(
     int64_t now) {
   Connection* oldest_needing_triggered_check = nullptr;
   for (auto conn : connections_) {
     if (!IsPingable(conn, now)) {
       continue;
     }
     bool needs_triggered_check =
         (!conn->writable() &&
@@ skipping 95 matching lines @@
 
   // During the initial state when nothing has been pinged yet, return the first
   // one in the ordered |connections_|.
   return *(std::find_if(connections_.begin(), connections_.end(),
                         [conn1, conn2](Connection* conn) {
                           return conn == conn1 || conn == conn2;
                         }));
 }
 
 }  // namespace cricket