refactor(connlib): batch resource change updates (#5575)

Currently, upon reconnecting, `snownet` returns a list of connection IDs
that have been closed. This was done to avoid emitting many identical
`ResourcesChanged` events. In all other events, `snownet` always only
references a single connection. To align this whilst not duplicating
`ResourcesChanged` events, we use a dedicated `bool` to check, whether
any of the events emitted by `snownet` require updating the clients
about our active resources.
This commit is contained in:
Thomas Eizinger
2024-06-27 17:48:41 +10:00
committed by GitHub
parent 18b9c35316
commit 58fad7cb2d
5 changed files with 25 additions and 29 deletions

View File

@@ -157,17 +157,20 @@ where
self.pending_events.clear();
let connections = self.connections.iter_ids().collect::<Vec<_>>();
let num_connections = connections.len();
let closed_connections = self
.connections
.iter_ids()
.map(Event::ConnectionClosed)
.collect::<Vec<_>>();
let num_connections = closed_connections.len();
self.pending_events
.push_back(Event::ConnectionsCleared(connections));
self.pending_events.extend(closed_connections);
self.host_candidates.clear();
self.connections.clear();
self.buffered_transmits.clear();
tracing::debug!("Cleared {num_connections} connections");
tracing::debug!(%num_connections, "Closed all connections as part of reconnecting");
}
pub fn public_key(&self) -> PublicKey {
@@ -1270,8 +1273,8 @@ pub enum Event<TId> {
/// All state associated with the connection has been cleared.
ConnectionFailed(TId),
/// The referenced connections had their state cleared.
ConnectionsCleared(Vec<TId>),
/// We closed a connection (e.g. due to inactivity, roaming, etc).
ConnectionClosed(TId),
}
#[derive(Clone, PartialEq)]

View File

@@ -634,7 +634,7 @@ impl<R> TestNode<R> {
.in_scope(|| other.node.remove_remote_candidate(connection, candidate)),
Event::ConnectionEstablished(_)
| Event::ConnectionFailed(_)
| Event::ConnectionsCleared(_) => {}
| Event::ConnectionClosed(_) => {}
};
}
}

View File

@@ -812,23 +812,13 @@ impl ClientState {
self.node.handle_timeout(now);
self.mangled_dns_queries.retain(|_, exp| now < *exp);
let mut resources_changed = false; // Track this separately to batch together `ResourcesChanged` events.
while let Some(event) = self.node.poll_event() {
match event {
snownet::Event::ConnectionFailed(id) => {
snownet::Event::ConnectionFailed(id) | snownet::Event::ConnectionClosed(id) => {
self.cleanup_connected_gateway(&id);
self.buffered_events
.push_back(ClientEvent::ResourcesChanged {
resources: self.resources(),
});
}
snownet::Event::ConnectionsCleared(ids) => {
for id in ids {
self.cleanup_connected_gateway(&id);
}
self.buffered_events
.push_back(ClientEvent::ResourcesChanged {
resources: self.resources(),
});
resources_changed = true;
}
snownet::Event::NewIceCandidate {
connection,
@@ -850,13 +840,17 @@ impl ClientState {
}),
snownet::Event::ConnectionEstablished(id) => {
self.update_site_status_by_gateway(&id, Status::Online);
self.buffered_events
.push_back(ClientEvent::ResourcesChanged {
resources: self.resources(),
});
resources_changed = true;
}
}
}
if resources_changed {
self.buffered_events
.push_back(ClientEvent::ResourcesChanged {
resources: self.resources(),
});
}
}
fn update_site_status_by_gateway(&mut self, gateway_id: &GatewayId, status: Status) {

View File

@@ -378,8 +378,7 @@ impl GatewayState {
candidate,
});
}
snownet::Event::ConnectionEstablished(_)
| snownet::Event::ConnectionsCleared(_) => {}
snownet::Event::ConnectionEstablished(_) | snownet::Event::ConnectionClosed(_) => {}
}
}
}

View File

@@ -400,7 +400,7 @@ impl<T> Eventloop<T> {
}
Some(
snownet::Event::InvalidateIceCandidate { .. }
| snownet::Event::ConnectionsCleared { .. },
| snownet::Event::ConnectionClosed { .. },
)
| None => {}
}