From e0cf16348694b8f54cb4b7db690fdd070366fe80 Mon Sep 17 00:00:00 2001 From: Charles Hall Date: Mon, 30 Sep 2024 20:25:23 -0700 Subject: [PATCH] delete useless admin commands To clear caches, restart the server. We may want to consider adding the cache sizes and database memory usage as metrics in the future. --- book/changelog.md | 8 ++++ src/database/abstraction.rs | 4 -- src/database/abstraction/rocksdb.rs | 25 ++--------- src/database/key_value/globals.rs | 70 ----------------------------- src/service.rs | 60 ------------------------- src/service/admin.rs | 37 --------------- src/service/globals/data.rs | 2 - 7 files changed, 11 insertions(+), 195 deletions(-) diff --git a/book/changelog.md b/book/changelog.md index 10bc1688..cb2e70af 100644 --- a/book/changelog.md +++ b/book/changelog.md @@ -73,6 +73,14 @@ This will be the first release of Grapevine since it was forked from Conduit ([!48](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/48)) 7. **BREAKING:** Remove unstable room versions. ([!59](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/59)) +8. Remove `memory-usage`, `clear-database-caches`, and `clear-service-caches` + admin commands. + ([!123](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/123)) + * The `memory-usage` command wasn't particularly useful since it can't + actually give you an accurate value in bytes and isn't supported on all + database backends. + * The latter two commands had poor UX and didn't have any noticable effect on + memory consumption. ### Changed diff --git a/src/database/abstraction.rs b/src/database/abstraction.rs index db14c728..63714cd3 100644 --- a/src/database/abstraction.rs +++ b/src/database/abstraction.rs @@ -20,10 +20,6 @@ pub(crate) trait KeyValueDatabaseEngine: Send + Sync { fn cleanup(&self) -> Result<()> { Ok(()) } - fn memory_usage(&self) -> Result { - Ok("Current database engine does not support memory usage reporting." - .to_owned()) - } } pub(crate) trait KvTree: Send + Sync { diff --git a/src/database/abstraction/rocksdb.rs b/src/database/abstraction/rocksdb.rs index 72e676a1..90cdb06a 100644 --- a/src/database/abstraction/rocksdb.rs +++ b/src/database/abstraction/rocksdb.rs @@ -6,10 +6,9 @@ use std::{ }; use rocksdb::{ - perf::get_memory_usage_stats, BlockBasedOptions, BoundColumnFamily, Cache, - ColumnFamilyDescriptor, DBCompactionStyle, DBCompressionType, - DBRecoveryMode, DBWithThreadMode, Direction, IteratorMode, MultiThreaded, - Options, ReadOptions, WriteOptions, + BlockBasedOptions, BoundColumnFamily, Cache, ColumnFamilyDescriptor, + DBCompactionStyle, DBCompressionType, DBRecoveryMode, DBWithThreadMode, + Direction, IteratorMode, MultiThreaded, Options, ReadOptions, WriteOptions, }; use tracing::Level; @@ -143,24 +142,6 @@ impl KeyValueDatabaseEngine for Arc { write_lock: RwLock::new(()), })) } - - #[allow(clippy::as_conversions, clippy::cast_precision_loss)] - fn memory_usage(&self) -> Result { - let stats = - get_memory_usage_stats(Some(&[&self.rocks]), Some(&[&self.cache]))?; - Ok(format!( - "Approximate memory usage of all the mem-tables: {:.3} \ - MB\nApproximate memory usage of un-flushed mem-tables: {:.3} \ - MB\nApproximate memory usage of all the table readers: {:.3} \ - MB\nApproximate memory usage by cache: {:.3} MB\nApproximate \ - memory usage by cache pinned: {:.3} MB\n", - stats.mem_table_total as f64 / 1024.0 / 1024.0, - stats.mem_table_unflushed as f64 / 1024.0 / 1024.0, - stats.mem_table_readers_total as f64 / 1024.0 / 1024.0, - stats.cache_total as f64 / 1024.0 / 1024.0, - self.cache.get_pinned_usage() as f64 / 1024.0 / 1024.0, - )) - } } impl RocksDbEngineTree<'_> { diff --git a/src/database/key_value/globals.rs b/src/database/key_value/globals.rs index 819ef98c..e3293571 100644 --- a/src/database/key_value/globals.rs +++ b/src/database/key_value/globals.rs @@ -1,8 +1,5 @@ -use std::collections::HashMap; - use async_trait::async_trait; use futures_util::{stream::FuturesUnordered, StreamExt}; -use lru_cache::LruCache; use ruma::{ api::federation::discovery::{OldVerifyKey, ServerSigningKeys}, signatures::Ed25519KeyPair, @@ -138,73 +135,6 @@ impl service::globals::Data for KeyValueDatabase { self.db.cleanup() } - fn memory_usage(&self) -> String { - let pdu_cache = self.pdu_cache.lock().unwrap().len(); - let shorteventid_cache = self.shorteventid_cache.lock().unwrap().len(); - let auth_chain_cache = self.auth_chain_cache.lock().unwrap().len(); - let eventidshort_cache = self.eventidshort_cache.lock().unwrap().len(); - let statekeyshort_cache = - self.statekeyshort_cache.lock().unwrap().len(); - let our_real_users_cache = - self.our_real_users_cache.read().unwrap().len(); - let appservice_in_room_cache = - self.appservice_in_room_cache.read().unwrap().len(); - let lasttimelinecount_cache = - self.lasttimelinecount_cache.lock().unwrap().len(); - - let mut response = format!( - "\ -pdu_cache: {pdu_cache} -shorteventid_cache: {shorteventid_cache} -auth_chain_cache: {auth_chain_cache} -eventidshort_cache: {eventidshort_cache} -statekeyshort_cache: {statekeyshort_cache} -our_real_users_cache: {our_real_users_cache} -appservice_in_room_cache: {appservice_in_room_cache} -lasttimelinecount_cache: {lasttimelinecount_cache}\n" - ); - if let Ok(db_stats) = self.db.memory_usage() { - response += &db_stats; - } - - response - } - - fn clear_caches(&self, amount: u32) { - if amount > 0 { - let c = &mut *self.pdu_cache.lock().unwrap(); - *c = LruCache::new(c.capacity()); - } - if amount > 1 { - let c = &mut *self.shorteventid_cache.lock().unwrap(); - *c = LruCache::new(c.capacity()); - } - if amount > 2 { - let c = &mut *self.auth_chain_cache.lock().unwrap(); - *c = LruCache::new(c.capacity()); - } - if amount > 3 { - let c = &mut *self.eventidshort_cache.lock().unwrap(); - *c = LruCache::new(c.capacity()); - } - if amount > 4 { - let c = &mut *self.statekeyshort_cache.lock().unwrap(); - *c = LruCache::new(c.capacity()); - } - if amount > 5 { - let c = &mut *self.our_real_users_cache.write().unwrap(); - *c = HashMap::new(); - } - if amount > 6 { - let c = &mut *self.appservice_in_room_cache.write().unwrap(); - *c = HashMap::new(); - } - if amount > 7 { - let c = &mut *self.lasttimelinecount_cache.lock().unwrap(); - *c = HashMap::new(); - } - } - fn load_keypair(&self) -> Result { let keypair_bytes = self.global.get(b"keypair")?.map_or_else( || { diff --git a/src/service.rs b/src/service.rs index c2dcb0f8..1c45fbcd 100644 --- a/src/service.rs +++ b/src/service.rs @@ -168,64 +168,4 @@ impl Services { "Services::install was called more than once" ); } - - async fn memory_usage(&self) -> String { - let lazy_load_waiting = - self.rooms.lazy_loading.lazy_load_waiting.lock().await.len(); - let server_visibility_cache = self - .rooms - .state_accessor - .server_visibility_cache - .lock() - .unwrap() - .len(); - let user_visibility_cache = self - .rooms - .state_accessor - .user_visibility_cache - .lock() - .unwrap() - .len(); - let stateinfo_cache = - self.rooms.state_compressor.stateinfo_cache.lock().unwrap().len(); - let roomid_spacechunk_cache = - self.rooms.spaces.roomid_spacechunk_cache.lock().await.len(); - - format!( - "\ -lazy_load_waiting: {lazy_load_waiting} -server_visibility_cache: {server_visibility_cache} -user_visibility_cache: {user_visibility_cache} -stateinfo_cache: {stateinfo_cache} -roomid_spacechunk_cache: {roomid_spacechunk_cache}" - ) - } - - async fn clear_caches(&self, amount: u32) { - if amount > 0 { - self.rooms.lazy_loading.lazy_load_waiting.lock().await.clear(); - } - if amount > 1 { - self.rooms - .state_accessor - .server_visibility_cache - .lock() - .unwrap() - .clear(); - } - if amount > 2 { - self.rooms - .state_accessor - .user_visibility_cache - .lock() - .unwrap() - .clear(); - } - if amount > 3 { - self.rooms.state_compressor.stateinfo_cache.lock().unwrap().clear(); - } - if amount > 5 { - self.rooms.spaces.roomid_spacechunk_cache.lock().await.clear(); - } - } } diff --git a/src/service/admin.rs b/src/service/admin.rs index e46368ed..8fd0abdf 100644 --- a/src/service/admin.rs +++ b/src/service/admin.rs @@ -141,21 +141,6 @@ enum AdminCommand { event_id: Box, }, - /// Print database memory usage statistics - MemoryUsage, - - /// Clears all of Grapevine's database caches with index smaller than the - /// amount - ClearDatabaseCaches { - amount: u32, - }, - - /// Clears all of Grapevine's service caches with index smaller than the - /// amount - ClearServiceCaches { - amount: u32, - }, - /// Reset user password ResetPassword { /// Username of the user for whom the password should be reset @@ -661,28 +646,6 @@ impl Service { } } } - AdminCommand::MemoryUsage => { - let response1 = services().memory_usage().await; - let response2 = services().globals.db.memory_usage(); - - RoomMessageEventContent::text_plain(format!( - "Services:\n{response1}\n\nDatabase:\n{response2}" - )) - } - AdminCommand::ClearDatabaseCaches { - amount, - } => { - services().globals.db.clear_caches(amount); - - RoomMessageEventContent::text_plain("Done.") - } - AdminCommand::ClearServiceCaches { - amount, - } => { - services().clear_caches(amount).await; - - RoomMessageEventContent::text_plain("Done.") - } AdminCommand::ResetPassword { username, } => { diff --git a/src/service/globals/data.rs b/src/service/globals/data.rs index 28e7e512..124e81b3 100644 --- a/src/service/globals/data.rs +++ b/src/service/globals/data.rs @@ -90,8 +90,6 @@ pub(crate) trait Data: Send + Sync { async fn watch(&self, user_id: &UserId, device_id: &DeviceId) -> Result<()>; fn cleanup(&self) -> Result<()>; - fn memory_usage(&self) -> String; - fn clear_caches(&self, amount: u32); fn load_keypair(&self) -> Result; fn remove_keypair(&self) -> Result<()>; /// Only extends the cached keys, not moving any verify_keys to