From 7f4ffead81b19675b85a410562fb41f299a44eee Mon Sep 17 00:00:00 2001 From: Hoyt Koepke Date: Wed, 18 Dec 2024 16:51:35 -0700 Subject: [PATCH 1/2] Update to ensure that drop issues a warning in debug mode instead of universally. --- mdb_shard/src/shard_file_manager.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/mdb_shard/src/shard_file_manager.rs b/mdb_shard/src/shard_file_manager.rs index e2a1acc..1ca3480 100644 --- a/mdb_shard/src/shard_file_manager.rs +++ b/mdb_shard/src/shard_file_manager.rs @@ -27,21 +27,11 @@ struct MDBShardFlushGuard { impl Drop for MDBShardFlushGuard { fn drop(&mut self) { - if self.shard.is_empty() { - return; - } - - if let Some(sd) = &self.session_directory { - // Check if the flushing directory exists. - if !sd.is_dir() { - error!("Error flushing reconstruction data on shutdown: {sd:?} is not a directory or doesn't exist"); - return; + if !self.shard.is_empty() { + // This is only supposed to happen on task cancellations, so we should + if cfg!(debug_assertions) { + eprintln!("[Debug] Warning: Shard dropped while data still present! This is an error outside of task cancellation."); } - - self.flush().unwrap_or_else(|e| { - error!("Error flushing reconstruction data on shutdown: {e:?}"); - None - }); } } } From 5fdf1f29fd17038169243c063d96a458cad94b45 Mon Sep 17 00:00:00 2001 From: Hoyt Koepke Date: Tue, 7 Jan 2025 14:24:13 -0800 Subject: [PATCH 2/2] Updated tests depending on Drop behavior. --- mdb_shard/src/shard_file_manager.rs | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/mdb_shard/src/shard_file_manager.rs b/mdb_shard/src/shard_file_manager.rs index 96e405f..433eefc 100644 --- a/mdb_shard/src/shard_file_manager.rs +++ b/mdb_shard/src/shard_file_manager.rs @@ -847,6 +847,7 @@ mod tests { let mut mdb = ShardFileManager::load_dir(tmp_dir.path(), false).await?; mdb.set_target_shard_min_size(T); // Set the targe shard size really low fill_with_random_shard(&mut mdb, &mut mdb_in_mem, 0, &[16; 16], &[16; 16]).await?; + mdb.flush().await?; } { let mut mdb = ShardFileManager::load_dir(tmp_dir.path(), false).await?; @@ -857,6 +858,8 @@ mod tests { fill_with_random_shard(&mut mdb, &mut mdb_in_mem, 1, &[25; 25], &[25; 25]).await?; verify_mdb_shards_match(&mdb, &mdb_in_mem, true).await?; + + mdb.flush().await?; } // Reload and verify @@ -1086,29 +1089,4 @@ mod tests { Ok(()) } - - #[tokio::test] - async fn test_teardown() -> Result<()> { - let tmp_dir = TempDir::new("gitxet_shard_test_1")?; - let mut mdb_in_mem = MDBInMemoryShard::default(); - - { - let mut mdb = ShardFileManager::load_dir(tmp_dir.path(), false).await?; - - fill_with_specific_shard(&mut mdb, &mut mdb_in_mem, &[(0, &[(11, 5)])], &[(100, &[(200, (0, 5))])]).await?; - - verify_mdb_shards_match(&mdb, &mdb_in_mem, true).await?; - // Note, no flush - } - - { - // Now, make sure that this happens if this directory is opened up - let mdb2 = ShardFileManager::load_dir(tmp_dir.path(), false).await?; - - // Make sure it's all in there this round. - verify_mdb_shards_match(&mdb2, &mdb_in_mem, true).await?; - } - - Ok(()) - } }