From 555847449b6ac9063278310c71030392bd308cb5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Jun 2022 18:02:17 +0200 Subject: [PATCH 1/3] Use `BTreeMap` in `Server` so we release memory when maps are cleared Co-Authored-By: Nathan Sobo --- crates/collab/src/rpc/store.rs | 17 +++++++---------- crates/rpc/src/peer.rs | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/collab/src/rpc/store.rs b/crates/collab/src/rpc/store.rs index 561b7c6ab4..d1eb4a3be6 100644 --- a/crates/collab/src/rpc/store.rs +++ b/crates/collab/src/rpc/store.rs @@ -1,10 +1,6 @@ use crate::db::{self, ChannelId, ProjectId, UserId}; use anyhow::{anyhow, Result}; -use collections::{ - btree_map, - hash_map::{self, Entry}, - BTreeMap, BTreeSet, HashMap, HashSet, -}; +use collections::{btree_map, hash_map::Entry, BTreeMap, BTreeSet, HashMap, HashSet}; use rpc::{proto, ConnectionId, Receipt}; use serde::Serialize; use std::{ @@ -18,11 +14,11 @@ use tracing::instrument; #[derive(Default, Serialize)] pub struct Store { - connections: HashMap, - connections_by_user_id: HashMap>, + connections: BTreeMap, + connections_by_user_id: BTreeMap>, projects: BTreeMap, #[serde(skip)] - channels: HashMap, + channels: BTreeMap, } #[derive(Serialize)] @@ -60,7 +56,7 @@ pub struct Worktree { pub root_name: String, pub visible: bool, #[serde(skip)] - pub entries: HashMap, + pub entries: BTreeMap, #[serde(skip)] pub extension_counts: HashMap, #[serde(skip)] @@ -210,7 +206,7 @@ impl Store { pub fn leave_channel(&mut self, connection_id: ConnectionId, channel_id: ChannelId) { if let Some(connection) = self.connections.get_mut(&connection_id) { connection.channels.remove(&channel_id); - if let hash_map::Entry::Occupied(mut entry) = self.channels.entry(channel_id) { + if let btree_map::Entry::Occupied(mut entry) = self.channels.entry(channel_id) { entry.get_mut().connection_ids.remove(&connection_id); if entry.get_mut().connection_ids.is_empty() { entry.remove(); @@ -596,6 +592,7 @@ impl Store { for worktree in project.worktrees.values_mut() { worktree.diagnostic_summaries.clear(); worktree.entries.clear(); + worktree.extension_counts.clear(); } } diff --git a/crates/rpc/src/peer.rs b/crates/rpc/src/peer.rs index 43dc2d2180..a22dcfe39e 100644 --- a/crates/rpc/src/peer.rs +++ b/crates/rpc/src/peer.rs @@ -24,7 +24,7 @@ use std::{ }; use tracing::instrument; -#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Serialize)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Serialize)] pub struct ConnectionId(pub u32); impl fmt::Display for ConnectionId { From a04adbcac1bfa371e82d61f8fd5a44ea0f6ddc44 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 24 Jun 2022 09:27:22 +0200 Subject: [PATCH 2/3] Don't trace message payload --- crates/collab/src/rpc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 9cbf2577e3..7e16d57c4c 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -228,8 +228,8 @@ impl Server { ); span.in_scope(|| { tracing::info!( - payload = format!("{:?}", envelope.payload).as_str(), - "message payload" + payload_type = envelope.payload_type_name(), + "message received" ); }); let future = (handler)(server, *envelope); From fc5517b6bed0f817705daba6ed7e1d77f8785d21 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 24 Jun 2022 09:28:52 +0200 Subject: [PATCH 3/3] Gather metrics only when `/metrics` endpoint is retrieved --- crates/collab/src/rpc.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 7e16d57c4c..79c1e53a0b 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1682,21 +1682,6 @@ impl<'a> Drop for StoreWriteGuard<'a> { fn drop(&mut self) { #[cfg(test)] self.check_invariants(); - - let metrics = self.metrics(); - - METRIC_CONNECTIONS.set(metrics.connections as _); - METRIC_REGISTERED_PROJECTS.set(metrics.registered_projects as _); - METRIC_ACTIVE_PROJECTS.set(metrics.active_projects as _); - METRIC_SHARED_PROJECTS.set(metrics.shared_projects as _); - - tracing::info!( - connections = metrics.connections, - registered_projects = metrics.registered_projects, - active_projects = metrics.active_projects, - shared_projects = metrics.shared_projects, - "metrics" - ); } } @@ -1802,7 +1787,11 @@ pub async fn handle_websocket_request( pub async fn handle_metrics(Extension(server): Extension>) -> axum::response::Response { // We call `store_mut` here for its side effects of updating metrics. - server.store_mut().await; + let metrics = server.store().await.metrics(); + METRIC_CONNECTIONS.set(metrics.connections as _); + METRIC_REGISTERED_PROJECTS.set(metrics.registered_projects as _); + METRIC_ACTIVE_PROJECTS.set(metrics.active_projects as _); + METRIC_SHARED_PROJECTS.set(metrics.shared_projects as _); let encoder = prometheus::TextEncoder::new(); let metric_families = prometheus::gather();