Skip to content

Commit 0c4795e

Browse files
authored
fix(storage): align DEL with generation lifecycle (#409)
* fix(storage): align DEL with generation lifecycle
1 parent 3164d4a commit 0c4795e

6 files changed

Lines changed: 726 additions & 102 deletions

File tree

src/storage/src/data_compaction_filter.rs

Lines changed: 77 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const DATA_FILTER_FACTORY_NAME: &std::ffi::CStr = c"DataCompactionFilterFactory"
4848
struct CompactionFilterTestState {
4949
entered: bool,
5050
released: bool,
51+
removed_keys: Vec<Vec<u8>>,
5152
}
5253

5354
#[cfg(test)]
@@ -88,26 +89,45 @@ impl CompactionFilterTestGate {
8889
self.changed.notify_all();
8990
}
9091

92+
pub(crate) fn wait_until_removed(&self, timeout: std::time::Duration) -> Vec<Vec<u8>> {
93+
let state = self
94+
.state
95+
.lock()
96+
.expect("compaction filter test gate mutex should not be poisoned");
97+
let (state, _) = self
98+
.changed
99+
.wait_timeout_while(state, timeout, |state| state.removed_keys.is_empty())
100+
.expect("compaction filter test gate wait should succeed");
101+
state.removed_keys.clone()
102+
}
103+
104+
fn record_removed_key(&self, key: &[u8]) {
105+
let Ok(mut state) = self.state.lock() else {
106+
return;
107+
};
108+
state.removed_keys.push(key.to_vec());
109+
self.changed.notify_all();
110+
}
111+
91112
fn enter_and_wait(&self, key: &[u8]) {
92113
if !key.starts_with(&self.key_prefix) {
93114
return;
94115
}
95116

96-
let mut state = self
97-
.state
98-
.lock()
99-
.expect("compaction filter test gate mutex should not be poisoned");
117+
let Ok(mut state) = self.state.lock() else {
118+
return;
119+
};
100120
if state.entered {
101121
return;
102122
}
103123

104124
state.entered = true;
105125
self.changed.notify_all();
106126
while !state.released {
107-
state = self
108-
.changed
109-
.wait(state)
110-
.expect("compaction filter test gate wait should succeed");
127+
let Ok(next_state) = self.changed.wait(state) else {
128+
return;
129+
};
130+
state = next_state;
111131
}
112132
}
113133
}
@@ -118,6 +138,12 @@ fn compaction_filter_test_gate() -> &'static Mutex<Option<Weak<CompactionFilterT
118138
GATE.get_or_init(|| Mutex::new(None))
119139
}
120140

141+
#[cfg(test)]
142+
pub(crate) fn compaction_filter_test_serial_mutex() -> &'static Mutex<()> {
143+
static MUTEX: OnceLock<Mutex<()>> = OnceLock::new();
144+
MUTEX.get_or_init(|| Mutex::new(()))
145+
}
146+
121147
#[cfg(test)]
122148
pub(crate) struct CompactionFilterTestGateGuard {
123149
gate: Arc<CompactionFilterTestGate>,
@@ -127,9 +153,9 @@ pub(crate) struct CompactionFilterTestGateGuard {
127153
impl Drop for CompactionFilterTestGateGuard {
128154
fn drop(&mut self) {
129155
self.gate.release();
130-
*compaction_filter_test_gate()
131-
.lock()
132-
.expect("compaction filter test gate registry should not be poisoned") = None;
156+
if let Ok(mut installed) = compaction_filter_test_gate().lock() {
157+
*installed = None;
158+
}
133159
}
134160
}
135161

@@ -139,7 +165,7 @@ pub(crate) fn install_compaction_filter_test_gate(
139165
) -> CompactionFilterTestGateGuard {
140166
let mut installed = compaction_filter_test_gate()
141167
.lock()
142-
.expect("compaction filter test gate registry should not be poisoned");
168+
.unwrap_or_else(std::sync::PoisonError::into_inner);
143169
assert!(
144170
installed.is_none(),
145171
"only one compaction test gate may be installed"
@@ -151,16 +177,26 @@ pub(crate) fn install_compaction_filter_test_gate(
151177

152178
#[cfg(test)]
153179
fn block_once_for_compaction_filter_test(key: &[u8]) {
154-
let gate = compaction_filter_test_gate()
155-
.lock()
156-
.expect("compaction filter test gate registry should not be poisoned")
157-
.as_ref()
158-
.and_then(Weak::upgrade);
180+
let Ok(installed) = compaction_filter_test_gate().lock() else {
181+
return;
182+
};
183+
let gate = installed.as_ref().and_then(Weak::upgrade);
159184
if let Some(gate) = gate {
160185
gate.enter_and_wait(key);
161186
}
162187
}
163188

189+
#[cfg(test)]
190+
fn record_compaction_filter_remove(key: &[u8]) {
191+
let Ok(installed) = compaction_filter_test_gate().lock() else {
192+
return;
193+
};
194+
let gate = installed.as_ref().and_then(Weak::upgrade);
195+
if let Some(gate) = gate {
196+
gate.record_removed_key(key);
197+
}
198+
}
199+
164200
#[derive(Debug)]
165201
enum MetaLookup {
166202
Valid,
@@ -326,21 +362,28 @@ impl CompactionFilter for DataCompactionFilter {
326362
return CompactionDecision::Keep;
327363
};
328364

329-
match self.ensure_meta_state(&meta_key) {
365+
let decision = match self.ensure_meta_state(&meta_key) {
330366
MetaLookup::Unavailable => CompactionDecision::Keep,
331367
MetaLookup::NotFound => CompactionDecision::Remove,
332368
MetaLookup::Valid => {
333369
let cur_time = Utc::now().timestamp_micros() as u64;
334370
if self.cur_meta_etime != 0 && self.cur_meta_etime < cur_time {
335-
return CompactionDecision::Remove;
336-
}
337-
338-
match Self::extract_data_version(key) {
339-
Some(ver) if self.cur_meta_version > ver => CompactionDecision::Remove,
340-
_ => CompactionDecision::Keep,
371+
CompactionDecision::Remove
372+
} else {
373+
match Self::extract_data_version(key) {
374+
Some(ver) if self.cur_meta_version > ver => CompactionDecision::Remove,
375+
_ => CompactionDecision::Keep,
376+
}
341377
}
342378
}
379+
};
380+
381+
#[cfg(test)]
382+
if let CompactionDecision::Remove = &decision {
383+
record_compaction_filter_remove(key);
343384
}
385+
386+
decision
344387
}
345388
}
346389

@@ -576,6 +619,9 @@ mod tests {
576619

577620
#[test]
578621
fn test_removes_data_if_meta_is_expired() {
622+
let _serial = compaction_filter_test_serial_mutex()
623+
.lock()
624+
.expect("compaction filter gate tests must run serially");
579625
let path = unique_test_db_path();
580626
let (db_cell, db) = setup_db_for_filter_test(&path);
581627

@@ -598,8 +644,15 @@ mod tests {
598644

599645
let data_key = encode_data_key(b"mykey", 1);
600646

647+
let gate = CompactionFilterTestGate::new(b"mykey");
648+
let _gate_guard = install_compaction_filter_test_gate(Arc::clone(&gate));
649+
601650
let decision = filter.filter(0, &data_key, b"");
602651
assert!(matches!(decision, CompactionDecision::Remove));
652+
assert_eq!(
653+
gate.wait_until_removed(std::time::Duration::from_secs(1)),
654+
vec![data_key]
655+
);
603656
}
604657

605658
#[test]

0 commit comments

Comments
 (0)