fix: dedup sql error when deleting many entries (#3300)

Description --- Fixes ERROR Error when trimming message dedup cache: ResultError(DatabaseError(__Unknown, "too many SQL variables")) Motivation and Context --- Bug fix when trimming many rows. Cleanup performance will also be better How Has This Been Tested? --- Existing test pass
tari-project · Sep 6, 2021 · 7e58845 · 7e58845
1 parent 3d49eae
commit 7e58845
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 15 deletions.
diff --git a/comms/dht/src/actor.rs b/comms/dht/src/actor.rs
@@ -854,7 +854,8 @@ mod test {
         let dedup_cache_db = actor.msg_hash_dedup_cache.clone();
         // The cleanup ticker starts when the actor is spawned; the first cleanup event will fire fairly soon after the
         // task is running on a thread. To remove this race condition, we trim the cache in the test.
-        dedup_cache_db.trim_entries().await.unwrap();
+        let num_trimmed = dedup_cache_db.trim_entries().await.unwrap();
+        assert_eq!(num_trimmed, 10);
         actor.spawn();
 
         // Verify that the last half of the signatures are still present in the cache

diff --git a/comms/dht/src/dedup/dedup_cache.rs b/comms/dht/src/dedup/dedup_cache.rs
@@ -25,7 +25,7 @@ use crate::{
     storage::{DbConnection, StorageError},
 };
 use chrono::{NaiveDateTime, Utc};
-use diesel::{dsl, result::DatabaseErrorKind, ExpressionMethods, OptionalExtension, QueryDsl, RunQueryDsl};
+use diesel::{dsl, result::DatabaseErrorKind, sql_types, ExpressionMethods, OptionalExtension, QueryDsl, RunQueryDsl};
 use log::*;
 use tari_comms::types::CommsPublicKey;
 use tari_crypto::tari_utilities::hex::Hex;
@@ -90,27 +90,22 @@ impl DedupCacheDatabase {
 
     /// Trims the dedup cache to the configured limit by removing the oldest entries
     pub async fn trim_entries(&self) -> Result<usize, StorageError> {
-        let capacity = self.capacity;
+        let capacity = self.capacity as i64;
         self.connection
             .with_connection_async(move |conn| {
                 let mut num_removed = 0;
                 let msg_count = dedup_cache::table
                     .select(dsl::count(dedup_cache::id))
-                    .first::<i64>(conn)? as usize;
+                    .first::<i64>(conn)?;
                 // Hysteresis added to minimize database impact
                 if msg_count > capacity {
                     let remove_count = msg_count - capacity;
-                    num_removed = diesel::delete(dedup_cache::table)
-                        .filter(
-                            dedup_cache::id.eq_any(
-                                dedup_cache::table
-                                    .select(dedup_cache::id)
-                                    .order_by(dedup_cache::last_hit_at.asc())
-                                    .limit(remove_count as i64)
-                                    .get_results::<i32>(conn)?,
-                            ),
-                        )
-                        .execute(conn)?;
+                    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)",
+                    )
+                    .bind::<sql_types::BigInt, _>(remove_count)
+                    .execute(conn)?;
                 }
                 debug!(
                     target: LOG_TARGET,