From 8c51252adfe0879e167320b0b69e50111e9f142a Mon Sep 17 00:00:00 2001 From: zwang28 <84491488@qq.com> Date: Sun, 29 Sep 2024 15:51:07 +0800 Subject: [PATCH] fix tests --- src/meta/src/hummock/manager/context.rs | 3 +- src/meta/src/hummock/manager/gc.rs | 16 ++------ src/meta/src/hummock/manager/tests.rs | 38 +++++++++++++++++++ .../hummock_test/src/hummock_storage_tests.rs | 1 + 4 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/meta/src/hummock/manager/context.rs b/src/meta/src/hummock/manager/context.rs index 2049a7736a953..164c112cadff0 100644 --- a/src/meta/src/hummock/manager/context.rs +++ b/src/meta/src/hummock/manager/context.rs @@ -242,12 +242,13 @@ impl HummockManager { } // TODO: add sanity check for serverless compaction + // sanity check to ensure SSTs to commit have not been full GCed yet. let now = self.now(); let sst_retention_watermark = now.saturating_sub(self.env.opts.min_sst_retention_time_sec); for sst in sstables { if sst.created_at < sst_retention_watermark { - return Err(anyhow::anyhow!("SST may have been GCed: SST timestamp {}, meta node timestamp {}, retention_sec {}, watermark {}", sst.created_at, now, self.env.opts.min_sst_retention_time_sec, sst_retention_watermark).into()); + return Err(anyhow::anyhow!("SST {} is rejected from being committed since it's below watermark: SST timestamp {}, meta node timestamp {}, retention_sec {}, watermark {}", sst.sst_info.sst_id, sst.created_at, now, self.env.opts.min_sst_retention_time_sec, sst_retention_watermark).into()); } } diff --git a/src/meta/src/hummock/manager/gc.rs b/src/meta/src/hummock/manager/gc.rs index 2f77ab387a650..bbb0114cf568f 100644 --- a/src/meta/src/hummock/manager/gc.rs +++ b/src/meta/src/hummock/manager/gc.rs @@ -183,7 +183,7 @@ impl HummockManager { prefix: Option, ) -> Result { self.metrics.full_gc_trigger_count.inc(); - // Set a minimum sst_retention_time to avoid deleting SSTs of on-going write op. + // Set a minimum sst_retention_time. let sst_retention_time = cmp::max( sst_retention_time, Duration::from_secs(self.env.opts.min_sst_retention_time_sec), @@ -419,17 +419,12 @@ mod tests { None ) .unwrap()); - let full_scan_task = match receiver.recv().await.unwrap().unwrap().event.unwrap() { + let _full_scan_task = match receiver.recv().await.unwrap().unwrap().event.unwrap() { ResponseEvent::FullScanTask(task) => task, _ => { panic!() } }; - // min_sst_retention_time_sec override user provided value. - assert_eq!( - hummock_manager.env.opts.min_sst_retention_time_sec, - full_scan_task.sst_retention_time_sec - ); assert!(hummock_manager .start_full_gc( @@ -437,17 +432,12 @@ mod tests { None ) .unwrap()); - let full_scan_task = match receiver.recv().await.unwrap().unwrap().event.unwrap() { + let _full_scan_task = match receiver.recv().await.unwrap().unwrap().event.unwrap() { ResponseEvent::FullScanTask(task) => task, _ => { panic!() } }; - // min_sst_retention_time_sec doesn't override user provided value. - assert_eq!( - hummock_manager.env.opts.min_sst_retention_time_sec + 1, - full_scan_task.sst_retention_time_sec - ); // Empty input results immediate return, without waiting heartbeat. hummock_manager diff --git a/src/meta/src/hummock/manager/tests.rs b/src/meta/src/hummock/manager/tests.rs index 9fa9b11a026cb..5a9aa15f7bee0 100644 --- a/src/meta/src/hummock/manager/tests.rs +++ b/src/meta/src/hummock/manager/tests.rs @@ -89,6 +89,7 @@ fn gen_local_sstable_info(sst_id: u64, table_ids: Vec, epoch: u64) -> Local LocalSstableInfo { sst_info: gen_sstable_info(sst_id, table_ids, epoch), table_stats: Default::default(), + created_at: u64::MAX, } } fn get_compaction_group_object_ids( @@ -794,6 +795,31 @@ async fn test_invalid_sst_id() { ); } + // reject due to SST's timestamp is below watermark + let ssts_below_watermerk = ssts + .iter() + .map(|s| LocalSstableInfo { + sst_info: s.sst_info.clone(), + table_stats: s.table_stats.clone(), + created_at: 0, + }) + .collect(); + let error = hummock_meta_client + .commit_epoch( + epoch, + SyncResult { + uncommitted_ssts: ssts_below_watermerk, + ..Default::default() + }, + false, + ) + .await + .unwrap_err(); + assert!(error + .as_report() + .to_string() + .contains("is rejected from being committed since it's below watermark")); + hummock_meta_client .commit_epoch( epoch, @@ -1275,6 +1301,7 @@ async fn test_version_stats() { .iter() .map(|table_id| (*table_id, table_stats_change.clone())) .collect(), + created_at: u64::MAX, }) .collect_vec(); hummock_meta_client @@ -1394,6 +1421,7 @@ async fn test_split_compaction_group_on_commit() { }, ), ]), + created_at: u64::MAX, }; hummock_meta_client .commit_epoch( @@ -1489,6 +1517,7 @@ async fn test_split_compaction_group_on_demand_basic() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; let sst_2 = LocalSstableInfo { sst_info: SstableInfo { @@ -1507,6 +1536,7 @@ async fn test_split_compaction_group_on_demand_basic() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; hummock_meta_client .commit_epoch( @@ -1598,6 +1628,7 @@ async fn test_split_compaction_group_on_demand_non_trivial() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; hummock_manager .register_table_ids_for_test(&[(100, 2), (101, 2)]) @@ -1694,6 +1725,7 @@ async fn test_split_compaction_group_trivial_expired() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; let sst_2 = LocalSstableInfo { sst_info: SstableInfo { @@ -1712,6 +1744,7 @@ async fn test_split_compaction_group_trivial_expired() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; let mut sst_3 = sst_2.clone(); let mut sst_4 = sst_1.clone(); @@ -1857,6 +1890,7 @@ async fn test_split_compaction_group_on_demand_bottom_levels() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; hummock_meta_client .commit_epoch( @@ -2019,6 +2053,7 @@ async fn test_compaction_task_expiration_due_to_split_group() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; let sst_2 = LocalSstableInfo { sst_info: SstableInfo { @@ -2037,6 +2072,7 @@ async fn test_compaction_task_expiration_due_to_split_group() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; hummock_meta_client @@ -2379,6 +2415,7 @@ async fn test_unregister_moved_table() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; let sst_2 = LocalSstableInfo { sst_info: SstableInfo { @@ -2397,6 +2434,7 @@ async fn test_unregister_moved_table() { ..Default::default() }, table_stats: Default::default(), + created_at: u64::MAX, }; hummock_meta_client diff --git a/src/storage/hummock_test/src/hummock_storage_tests.rs b/src/storage/hummock_test/src/hummock_storage_tests.rs index c477693b114fb..13ee6f68cf76e 100644 --- a/src/storage/hummock_test/src/hummock_storage_tests.rs +++ b/src/storage/hummock_test/src/hummock_storage_tests.rs @@ -2607,6 +2607,7 @@ async fn test_commit_multi_epoch() { }) .collect(), sst_info: sst, + created_at: u64::MAX, }], new_table_fragment_info, change_log_delta: Default::default(),