Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

raftstore-v2: not update modification index for delete range #14905

Merged
merged 10 commits into from Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions components/raftstore-v2/src/operation/command/write/mod.rs
Expand Up @@ -33,6 +33,8 @@ pub use raftstore::store::simple_write::{
SimpleWrite, SimpleWriteBinary, SimpleWriteEncoder, SimpleWriteReqDecoder,
};

const MAGIC_KEY: &str = "!magic_delete";

impl<EK: KvEngine, ER: RaftEngine> Peer<EK, ER> {
#[inline]
pub fn on_simple_write<T>(
Expand Down Expand Up @@ -313,6 +315,35 @@ impl<EK: KvEngine, R: ApplyResReporter> Apply<EK, R> {
// .unwrap_or_else(move |e| fail_f(e,
// DeleteStrategy::DeleteBlobs));
}

self.ensure_write_buffer();
// Delete range may not fill anything in memtable if the range is fit well with
SpadeA-Tang marked this conversation as resolved.
Show resolved Hide resolved
// sst files. If no further write/delete is performaned for this cf, flushed
// index and modified index in apply trace will never matched, which makes admin
// flush index never progress. It severely impacts raft log gc and raft log
// replay.
// So artificially add a magic delete here to make at least one element will be
// put in memtable.
let res = if cf.is_empty() || cf == CF_DEFAULT {
self.write_batch
.as_mut()
.unwrap()
.delete(MAGIC_KEY.as_bytes())
SpadeA-Tang marked this conversation as resolved.
Show resolved Hide resolved
} else {
self.write_batch
.as_mut()
.unwrap()
.delete_cf(cf, MAGIC_KEY.as_bytes())
};
res.unwrap_or_else(|e| {
slog_panic!(
self.logger,
"failed to delete magic in delete range";
"cf" => cf,
"error" => ?e
);
});

if index != u64::MAX {
self.modifications_mut()[off] = index;
}
Expand Down
1 change: 1 addition & 0 deletions tests/Cargo.toml
Expand Up @@ -94,6 +94,7 @@ protobuf = { version = "2.8", features = ["bytes"] }
raft = { version = "0.7.0", default-features = false, features = ["protobuf-codec"] }
raft_log_engine = { workspace = true }
raftstore = { workspace = true }
raftstore-v2 = { workspace = true }
rand = "0.8.3"
resource_control = { workspace = true }
slog = { workspace = true }
Expand Down
1 change: 1 addition & 0 deletions tests/failpoints/cases/mod.rs
@@ -1,5 +1,6 @@
// Copyright 2017 TiKV Project Authors. Licensed under Apache-2.0.

mod test_apply_trace;
mod test_async_fetch;
mod test_async_io;
mod test_backup;
Expand Down
36 changes: 36 additions & 0 deletions tests/failpoints/cases/test_apply_trace.rs
@@ -0,0 +1,36 @@
// Copyright 2023 TiKV Project Authors. Licensed under Apache-2.0.

use std::time::Duration;

use engine_traits::{
MiscExt, RaftEngineReadOnly, CF_DEFAULT, CF_LOCK, CF_RAFT, CF_WRITE, DATA_CFS,
};

#[test]
fn test_flush_before_stop2() {
use test_raftstore_v2::*;

let mut cluster = new_server_cluster(0, 3);
cluster.run();

for i in 0..100 {
let key = format!("k{:03}", i);
cluster.must_put_cf(CF_WRITE, key.as_bytes(), b"val");
cluster.must_put_cf(CF_LOCK, key.as_bytes(), b"val");
}

cluster.must_delete_range_cf(CF_DEFAULT, b"k000", b"k020");
cluster.must_delete_range_cf(CF_DEFAULT, b"k020", b"k040");

let raft_engine = cluster.get_raft_engine(1);
let mut cache = cluster.engines[0].0.get(1).unwrap();
let tablet = cache.latest().unwrap();
tablet.flush_cfs(DATA_CFS, true).unwrap();

// wait for persist admin flush index
std::thread::sleep(Duration::from_secs(5));

let admin_flush = raft_engine.get_flushed_index(1, CF_RAFT).unwrap().unwrap();
println!("region_id {}, index {:?}", 1, admin_flush);
assert!(admin_flush > 200);
}