Skip to content

Commit fa53ff1

Browse files
authored
fix(meta-service): detach the SysData to avoid race condition (#18722)
* fix(meta-service): detach the SysData to avoid race condition When creating a new level in state-machine, it should detach the SysData to avoid race condition with snapshot building. Before this commit, the new writable level and the snapshot compactor shares the same data thus the new applied data increases the `last-log-id` of a new built snapshot. Result in a snapshot that lacks some log entries it declares to have. * M src/meta/raft-store/src/sm_v003/compact_immutable_levels_test.rs
1 parent 9451238 commit fa53ff1

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

src/meta/raft-store/src/leveled_store/level.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ impl Level {
108108
pub(crate) fn new_level(&self) -> Self {
109109
let level = Self {
110110
// sys data are cloned
111-
sys_data: self.sys_data.clone(),
111+
sys_data: Arc::new(Mutex::new(self.sys_data.lock().unwrap().clone())),
112112

113113
// Large data set is referenced.
114114
kv: Default::default(),
@@ -147,3 +147,23 @@ impl Level {
147147
self.with_sys_data(|s| s.nodes_mut().clone())
148148
}
149149
}
150+
151+
#[cfg(test)]
152+
mod tests {
153+
use super::*;
154+
155+
#[test]
156+
fn test_new_level_detach_sys_data() {
157+
let level1 = Level::default();
158+
let level2 = level1.new_level();
159+
level1.with_sys_data(|s: &mut SysData| {
160+
s.update_seq(1);
161+
});
162+
assert_eq!(level1.with_sys_data(|s| s.curr_seq()), 1);
163+
164+
level2.with_sys_data(|s: &mut SysData| {
165+
s.update_seq(10);
166+
});
167+
assert_eq!(level1.with_sys_data(|s| s.curr_seq()), 1);
168+
}
169+
}

src/meta/raft-store/src/sm_v003/compact_immutable_levels_test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ async fn test_compact_3_level() -> anyhow::Result<()> {
150150

151151
let (sys_data, strm) = compacting_data.compact_into_stream().await?;
152152
assert_eq!(
153-
r#"{"last_applied":{"leader_id":{"term":3,"node_id":3},"index":3},"last_membership":{"log_id":{"leader_id":{"term":3,"node_id":3},"index":3},"membership":{"configs":[],"nodes":{}}},"nodes":{"3":{"name":"3","endpoint":{"addr":"3","port":3},"grpc_api_advertise_address":null}},"sequence":7,"data_seq":3}"#,
153+
r#"{"last_applied":{"leader_id":{"term":3,"node_id":3},"index":3},"last_membership":{"log_id":{"leader_id":{"term":3,"node_id":3},"index":3},"membership":{"configs":[],"nodes":{}}},"nodes":{"3":{"name":"3","endpoint":{"addr":"3","port":3},"grpc_api_advertise_address":null}},"sequence":7,"data_seq":2}"#,
154154
serde_json::to_string(&sys_data).unwrap()
155155
);
156156

@@ -182,7 +182,7 @@ async fn test_export_2_level_with_meta() -> anyhow::Result<()> {
182182
.await?;
183183

184184
assert_eq!(
185-
r#"{"last_applied":null,"last_membership":{"log_id":null,"membership":{"configs":[],"nodes":{}}},"nodes":{},"sequence":4,"data_seq":2}"#,
185+
r#"{"last_applied":null,"last_membership":{"log_id":null,"membership":{"configs":[],"nodes":{}}},"nodes":{},"sequence":4,"data_seq":1}"#,
186186
serde_json::to_string(&sys_data).unwrap()
187187
);
188188

0 commit comments

Comments
 (0)