From 0163a2d868d6539d48799531a15a50f246879579 Mon Sep 17 00:00:00 2001 From: Hansie Odendaal Date: Mon, 22 Nov 2021 08:34:02 +0200 Subject: [PATCH] Review comments --- comms/dht/src/dedup/dedup_cache.rs | 59 +++++++++------------ comms/dht/src/store_forward/database/mod.rs | 32 +++++------ 2 files changed, 36 insertions(+), 55 deletions(-) diff --git a/comms/dht/src/dedup/dedup_cache.rs b/comms/dht/src/dedup/dedup_cache.rs index d56cfbf90a..b6b2d9338c 100644 --- a/comms/dht/src/dedup/dedup_cache.rs +++ b/comms/dht/src/dedup/dedup_cache.rs @@ -85,16 +85,13 @@ impl DedupCacheDatabase { pub fn trim_entries(&self) -> Result { let capacity = self.capacity as i64; let mut num_removed = 0; - let msg_count = { - let conn = self.connection.get_pooled_connection()?; - dedup_cache::table - .select(dsl::count(dedup_cache::id)) - .first::(&conn)? - }; + let conn = self.connection.get_pooled_connection()?; + let msg_count = dedup_cache::table + .select(dsl::count(dedup_cache::id)) + .first::(&conn)?; // Hysteresis added to minimize database impact if msg_count > capacity { let remove_count = msg_count - capacity; - let conn = self.connection.get_pooled_connection()?; num_removed = diesel::sql_query( "DELETE FROM dedup_cache WHERE id IN (SELECT id FROM dedup_cache ORDER BY last_hit_at ASC LIMIT $1)", ) @@ -110,17 +107,15 @@ impl DedupCacheDatabase { /// Insert new row into the table or updates an existing row. Returns the number of hits for this body hash. fn insert_body_hash_or_update_stats(&self, body_hash: String, public_key: String) -> Result { - let insert_result = { - let conn = self.connection.get_pooled_connection()?; - diesel::insert_into(dedup_cache::table) - .values(( - dedup_cache::body_hash.eq(&body_hash), - dedup_cache::sender_public_key.eq(&public_key), - dedup_cache::number_of_hits.eq(1), - dedup_cache::last_hit_at.eq(Utc::now().naive_utc()), - )) - .execute(&conn) - }; + let conn = self.connection.get_pooled_connection()?; + let insert_result = diesel::insert_into(dedup_cache::table) + .values(( + dedup_cache::body_hash.eq(&body_hash), + dedup_cache::sender_public_key.eq(&public_key), + dedup_cache::number_of_hits.eq(1), + dedup_cache::last_hit_at.eq(Utc::now().naive_utc()), + )) + .execute(&conn); match insert_result { Ok(1) => Ok(1), Ok(n) => Err(StorageError::UnexpectedResult(format!( @@ -130,25 +125,19 @@ impl DedupCacheDatabase { Err(diesel::result::Error::DatabaseError(kind, e_info)) => match kind { DatabaseErrorKind::UniqueViolation => { // Update hit stats for the message - { - let conn = self.connection.get_pooled_connection()?; - diesel::update(dedup_cache::table.filter(dedup_cache::body_hash.eq(&body_hash))) - .set(( - dedup_cache::sender_public_key.eq(&public_key), - dedup_cache::number_of_hits.eq(dedup_cache::number_of_hits + 1), - dedup_cache::last_hit_at.eq(Utc::now().naive_utc()), - )) - .execute(&conn)?; - } + diesel::update(dedup_cache::table.filter(dedup_cache::body_hash.eq(&body_hash))) + .set(( + dedup_cache::sender_public_key.eq(&public_key), + dedup_cache::number_of_hits.eq(dedup_cache::number_of_hits + 1), + dedup_cache::last_hit_at.eq(Utc::now().naive_utc()), + )) + .execute(&conn)?; // TODO: Diesel support for RETURNING statements would remove this query, but is not // TODO: available for Diesel + SQLite yet - let hits = { - let conn = self.connection.get_pooled_connection()?; - dedup_cache::table - .select(dedup_cache::number_of_hits) - .filter(dedup_cache::body_hash.eq(&body_hash)) - .get_result::(&conn)? - }; + let hits = dedup_cache::table + .select(dedup_cache::number_of_hits) + .filter(dedup_cache::body_hash.eq(&body_hash)) + .get_result::(&conn)?; Ok(hits as u32) }, diff --git a/comms/dht/src/store_forward/database/mod.rs b/comms/dht/src/store_forward/database/mod.rs index dc11b629ea..74a004fb97 100644 --- a/comms/dht/src/store_forward/database/mod.rs +++ b/comms/dht/src/store_forward/database/mod.rs @@ -203,28 +203,20 @@ impl StoreAndForwardDatabase { pub(crate) fn truncate_messages(&self, max_size: usize) -> Result { let mut num_removed = 0; - let msg_count = { - let conn = self.connection.get_pooled_connection()?; - stored_messages::table - .select(dsl::count(stored_messages::id)) - .first::(&conn)? as usize - }; + let conn = self.connection.get_pooled_connection()?; + let msg_count = stored_messages::table + .select(dsl::count(stored_messages::id)) + .first::(&conn)? as usize; if msg_count > max_size { let remove_count = msg_count - max_size; - let message_ids: Vec = { - let conn = self.connection.get_pooled_connection()?; - stored_messages::table - .select(stored_messages::id) - .order_by(stored_messages::stored_at.asc()) - .limit(remove_count as i64) - .get_results(&conn)? - }; - num_removed = { - let conn = self.connection.get_pooled_connection()?; - diesel::delete(stored_messages::table) - .filter(stored_messages::id.eq_any(message_ids)) - .execute(&conn)? - }; + let message_ids: Vec = stored_messages::table + .select(stored_messages::id) + .order_by(stored_messages::stored_at.asc()) + .limit(remove_count as i64) + .get_results(&conn)?; + num_removed = diesel::delete(stored_messages::table) + .filter(stored_messages::id.eq_any(message_ids)) + .execute(&conn)?; } Ok(num_removed) }