Skip to content

Commit

Permalink
gc_worker: disable gc if have negative ratio (#13904)
Browse files Browse the repository at this point in the history
close #13909, ref pingcap/tidb#39602

GC would be skipped once the `ratio_threshold` is negative or infinity.

Signed-off-by: hillium <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
  • Loading branch information
YuJuncen and ti-chi-bot authored Dec 8, 2022
1 parent 3122786 commit 3e0b8dd
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
16 changes: 16 additions & 0 deletions src/server/gc_worker/compaction_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,15 @@ pub fn check_need_gc(
context: &CompactionFilterContext,
) -> bool {
let check_props = |props: &MvccProperties| -> (bool, bool /* skip_more_checks */) {
// Disable GC directly once the config is negative or +inf.
// Disabling GC is useful in some abnormal scenarios where the transaction model
// would be break (e.g. writes with higher commit TS would be written BEFORE
// writes with lower commit TS, or write data with TS lower than current GC safe
// point). Use this at your own risk.
if ratio_threshold.is_sign_negative() || ratio_threshold.is_infinite() {
return (false, false);
}

if props.min_ts > safe_point {
return (false, false);
}
Expand Down Expand Up @@ -970,6 +979,13 @@ pub mod tests {
let default_key = Key::from_encoded_slice(b"zkey").append_ts(100.into());
let default_key = default_key.into_encoded();
assert!(raw_engine.get_value(&default_key).unwrap().is_none());

// If the ratio threshold is less than 0, GC would be skipped.
must_prewrite_put(&mut engine, b"zkey", &value, b"zkey", 210);
must_commit(&mut engine, b"zkey", 210, 220);
gc_runner.ratio_threshold = Some(-1.0);
gc_runner.safe_point(256).gc(&raw_engine);
must_get(&mut engine, b"zkey", 210, &value);
}

// Test dirty versions before a deletion mark can be handled correctly.
Expand Down
16 changes: 16 additions & 0 deletions src/server/gc_worker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ pub use crate::storage::{Callback, Error, ErrorInner, Result};
// Returns true if it needs gc.
// This is for optimization purpose, does not mean to be accurate.
fn check_need_gc(safe_point: TimeStamp, ratio_threshold: f64, props: &MvccProperties) -> bool {
// Disable GC directly once the config is negative or +inf.
// Disabling GC is useful in some abnormal scenarios where the transaction model
// would be break (e.g. writes with higher commit TS would be written BEFORE
// writes with lower commit TS, or write data with TS lower than current GC safe
// point). Use this at your own risk.
if ratio_threshold.is_sign_negative() || ratio_threshold.is_infinite() {
return false;
}
// Always GC.
if ratio_threshold < 1.0 {
return true;
Expand Down Expand Up @@ -77,6 +85,14 @@ mod tests {
props
}

#[test]
fn test_check_need_gc() {
let props = MvccProperties::default();
assert!(!check_need_gc(TimeStamp::max(), -1.0, &props));
assert!(!check_need_gc(TimeStamp::max(), f64::INFINITY, &props));
assert!(check_need_gc(TimeStamp::max(), 0.9, &props));
}

#[test]
fn test_need_gc() {
let path = tempfile::Builder::new()
Expand Down
2 changes: 1 addition & 1 deletion tests/failpoints/cases/test_table_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn test_check_need_gc() {

// Set ratio_threshold, let (props.num_versions as f64 > props.num_rows as
// f64 * ratio_threshold) return true
gc_runner.ratio_threshold = Option::Some(f64::MIN);
gc_runner.ratio_threshold = Option::Some(0.0f64);

// is_bottommost_level = false
do_gc(&raw_engine, 1, &mut gc_runner, &dir);
Expand Down

0 comments on commit 3e0b8dd

Please sign in to comment.