Skip to content

Commit

Permalink
fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
zwang28 committed Sep 29, 2024
1 parent d338210 commit 8c51252
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 14 deletions.
3 changes: 2 additions & 1 deletion src/meta/src/hummock/manager/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand Down
16 changes: 3 additions & 13 deletions src/meta/src/hummock/manager/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl HummockManager {
prefix: Option<String>,
) -> Result<bool> {
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),
Expand Down Expand Up @@ -419,35 +419,25 @@ 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(
Duration::from_secs(hummock_manager.env.opts.min_sst_retention_time_sec + 1),
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
Expand Down
38 changes: 38 additions & 0 deletions src/meta/src/hummock/manager/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ fn gen_local_sstable_info(sst_id: u64, table_ids: Vec<u32>, 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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1394,6 +1421,7 @@ async fn test_split_compaction_group_on_commit() {
},
),
]),
created_at: u64::MAX,
};
hummock_meta_client
.commit_epoch(
Expand Down Expand Up @@ -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 {
Expand All @@ -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(
Expand Down Expand Up @@ -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)])
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -2397,6 +2434,7 @@ async fn test_unregister_moved_table() {
..Default::default()
},
table_stats: Default::default(),
created_at: u64::MAX,
};

hummock_meta_client
Expand Down
1 change: 1 addition & 0 deletions src/storage/hummock_test/src/hummock_storage_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 8c51252

Please sign in to comment.