Skip to content

Commit cba6592

Browse files
Ligh0x74skyzh
authored and
yuoknow
committed
fix: handle the exclude boundary logic of the memory table (skyzh#140)
* fix: handle the exclude boundary logic of the memory table * add comments Signed-off-by: Alex Chi <[email protected]> --------- Signed-off-by: Alex Chi <[email protected]> Co-authored-by: Alex Chi <[email protected]>
1 parent 2280b5c commit cba6592

File tree

6 files changed

+95
-19
lines changed

6 files changed

+95
-19
lines changed

‎mini-lsm-mvcc/src/lsm_storage.rs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -805,15 +805,10 @@ impl LsmStorageInner {
805805
}; // drop global lock here
806806

807807
let mut memtable_iters = Vec::with_capacity(snapshot.imm_memtables.len() + 1);
808-
memtable_iters.push(Box::new(snapshot.memtable.scan(
809-
map_key_bound_plus_ts(lower, key::TS_RANGE_BEGIN),
810-
map_key_bound_plus_ts(upper, key::TS_RANGE_END),
811-
)));
808+
let (begin, end) = map_key_bound_plus_ts(lower, upper, read_ts);
809+
memtable_iters.push(Box::new(snapshot.memtable.scan(begin, end)));
812810
for memtable in snapshot.imm_memtables.iter() {
813-
memtable_iters.push(Box::new(memtable.scan(
814-
map_key_bound_plus_ts(lower, key::TS_RANGE_BEGIN),
815-
map_key_bound_plus_ts(upper, key::TS_RANGE_END),
816-
)));
811+
memtable_iters.push(Box::new(memtable.scan(begin, end)));
817812
}
818813
let memtable_iter = MergeIterator::create(memtable_iters);
819814

@@ -836,6 +831,8 @@ impl LsmStorageInner {
836831
table,
837832
KeySlice::from_slice(key, key::TS_RANGE_BEGIN),
838833
)?;
834+
// TODO: we can implement `key.next()` so that we can directly seek to the
835+
// right place in the previous line.
839836
while iter.is_valid() && iter.key().key_ref() == key {
840837
iter.next()?;
841838
}

‎mini-lsm-mvcc/src/mem_table.rs‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use crossbeam_skiplist::map::Entry;
2424
use ouroboros::self_referencing;
2525

2626
use crate::iterators::StorageIterator;
27-
use crate::key::{KeyBytes, KeySlice, TS_DEFAULT};
27+
use crate::key::{KeyBytes, KeySlice, TS_DEFAULT, TS_RANGE_BEGIN, TS_RANGE_END};
2828
use crate::table::SsTableBuilder;
2929
use crate::wal::Wal;
3030

@@ -63,13 +63,29 @@ pub(crate) fn map_key_bound(bound: Bound<KeySlice>) -> Bound<KeyBytes> {
6363
}
6464
}
6565

66-
/// Create a bound of `Bytes` from a bound of `KeySlice`.
67-
pub(crate) fn map_key_bound_plus_ts(bound: Bound<&[u8]>, ts: u64) -> Bound<KeySlice> {
68-
match bound {
69-
Bound::Included(x) => Bound::Included(KeySlice::from_slice(x, ts)),
70-
Bound::Excluded(x) => Bound::Excluded(KeySlice::from_slice(x, ts)),
71-
Bound::Unbounded => Bound::Unbounded,
72-
}
66+
/// Create a bound of `KeySlice` from a bound of `&[u8]`.
67+
pub(crate) fn map_key_bound_plus_ts<'a>(
68+
lower: Bound<&'a [u8]>,
69+
upper: Bound<&'a [u8]>,
70+
ts: u64,
71+
) -> (Bound<KeySlice<'a>>, Bound<KeySlice<'a>>) {
72+
(
73+
match lower {
74+
Bound::Included(x) => Bound::Included(KeySlice::from_slice(x, ts)),
75+
Bound::Excluded(x) => Bound::Excluded(KeySlice::from_slice(x, TS_RANGE_END)),
76+
Bound::Unbounded => Bound::Unbounded,
77+
},
78+
match upper {
79+
Bound::Included(x) => {
80+
// Note that we order the ts descending, but for a MVCC scan, we need all the history
81+
// so that we can access the latest key in case it is not updated in the current ts.
82+
// Therefore, we need to scan all the way to ts 0.
83+
Bound::Included(KeySlice::from_slice(x, TS_RANGE_END))
84+
}
85+
Bound::Excluded(x) => Bound::Excluded(KeySlice::from_slice(x, TS_RANGE_BEGIN)),
86+
Bound::Unbounded => Bound::Unbounded,
87+
},
88+
)
7389
}
7490

7591
impl MemTable {
@@ -127,8 +143,8 @@ impl MemTable {
127143
upper: Bound<&[u8]>,
128144
) -> MemTableIterator {
129145
self.scan(
130-
map_key_bound_plus_ts(lower, TS_DEFAULT),
131-
map_key_bound_plus_ts(upper, TS_DEFAULT),
146+
lower.map(|x| KeySlice::from_slice(x, TS_DEFAULT)),
147+
upper.map(|x| KeySlice::from_slice(x, TS_DEFAULT)),
132148
)
133149
}
134150

‎mini-lsm-mvcc/src/tests/week3_day3.rs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ fn test_task2_memtable_mvcc() {
4949
(Bytes::from("b"), Bytes::from("1")),
5050
],
5151
);
52+
check_lsm_iter_result_by_key(
53+
&mut snapshot1
54+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"b"))
55+
.unwrap(),
56+
vec![],
57+
);
5258
assert_eq!(snapshot2.get(b"a").unwrap(), Some(Bytes::from_static(b"2")));
5359
assert_eq!(snapshot2.get(b"b").unwrap(), Some(Bytes::from_static(b"1")));
5460
assert_eq!(snapshot2.get(b"c").unwrap(), None);
@@ -59,6 +65,12 @@ fn test_task2_memtable_mvcc() {
5965
(Bytes::from("b"), Bytes::from("1")),
6066
],
6167
);
68+
check_lsm_iter_result_by_key(
69+
&mut snapshot2
70+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"b"))
71+
.unwrap(),
72+
vec![],
73+
);
6274
assert_eq!(snapshot3.get(b"a").unwrap(), Some(Bytes::from_static(b"2")));
6375
assert_eq!(snapshot3.get(b"b").unwrap(), None);
6476
assert_eq!(snapshot3.get(b"c").unwrap(), Some(Bytes::from_static(b"1")));
@@ -69,6 +81,12 @@ fn test_task2_memtable_mvcc() {
6981
(Bytes::from("c"), Bytes::from("1")),
7082
],
7183
);
84+
check_lsm_iter_result_by_key(
85+
&mut snapshot3
86+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"c"))
87+
.unwrap(),
88+
vec![],
89+
);
7290
storage
7391
.inner
7492
.force_freeze_memtable(&storage.inner.state_lock.lock())
@@ -91,6 +109,12 @@ fn test_task2_memtable_mvcc() {
91109
(Bytes::from("b"), Bytes::from("1")),
92110
],
93111
);
112+
check_lsm_iter_result_by_key(
113+
&mut snapshot1
114+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"b"))
115+
.unwrap(),
116+
vec![],
117+
);
94118
assert_eq!(snapshot2.get(b"a").unwrap(), Some(Bytes::from_static(b"2")));
95119
assert_eq!(snapshot2.get(b"b").unwrap(), Some(Bytes::from_static(b"1")));
96120
assert_eq!(snapshot2.get(b"c").unwrap(), None);
@@ -101,6 +125,12 @@ fn test_task2_memtable_mvcc() {
101125
(Bytes::from("b"), Bytes::from("1")),
102126
],
103127
);
128+
check_lsm_iter_result_by_key(
129+
&mut snapshot2
130+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"b"))
131+
.unwrap(),
132+
vec![],
133+
);
104134
assert_eq!(snapshot3.get(b"a").unwrap(), Some(Bytes::from_static(b"2")));
105135
assert_eq!(snapshot3.get(b"b").unwrap(), None);
106136
assert_eq!(snapshot3.get(b"c").unwrap(), Some(Bytes::from_static(b"1")));
@@ -111,6 +141,12 @@ fn test_task2_memtable_mvcc() {
111141
(Bytes::from("c"), Bytes::from("1")),
112142
],
113143
);
144+
check_lsm_iter_result_by_key(
145+
&mut snapshot3
146+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"c"))
147+
.unwrap(),
148+
vec![],
149+
);
114150
assert_eq!(snapshot4.get(b"a").unwrap(), Some(Bytes::from_static(b"3")));
115151
assert_eq!(snapshot4.get(b"b").unwrap(), Some(Bytes::from_static(b"3")));
116152
assert_eq!(snapshot4.get(b"c").unwrap(), Some(Bytes::from_static(b"1")));
@@ -122,6 +158,12 @@ fn test_task2_memtable_mvcc() {
122158
(Bytes::from("c"), Bytes::from("1")),
123159
],
124160
);
161+
check_lsm_iter_result_by_key(
162+
&mut snapshot4
163+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"c"))
164+
.unwrap(),
165+
vec![(Bytes::from("b"), Bytes::from("3"))],
166+
);
125167
assert_eq!(snapshot5.get(b"a").unwrap(), Some(Bytes::from_static(b"4")));
126168
assert_eq!(snapshot5.get(b"b").unwrap(), Some(Bytes::from_static(b"3")));
127169
assert_eq!(snapshot5.get(b"c").unwrap(), Some(Bytes::from_static(b"1")));
@@ -133,6 +175,12 @@ fn test_task2_memtable_mvcc() {
133175
(Bytes::from("c"), Bytes::from("1")),
134176
],
135177
);
178+
check_lsm_iter_result_by_key(
179+
&mut snapshot5
180+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"c"))
181+
.unwrap(),
182+
vec![(Bytes::from("b"), Bytes::from("3"))],
183+
);
136184
assert_eq!(snapshot6.get(b"a").unwrap(), Some(Bytes::from_static(b"4")));
137185
assert_eq!(snapshot6.get(b"b").unwrap(), None);
138186
assert_eq!(snapshot6.get(b"c").unwrap(), Some(Bytes::from_static(b"5")));
@@ -143,6 +191,12 @@ fn test_task2_memtable_mvcc() {
143191
(Bytes::from("c"), Bytes::from("5")),
144192
],
145193
);
194+
check_lsm_iter_result_by_key(
195+
&mut snapshot6
196+
.scan(Bound::Excluded(b"a"), Bound::Excluded(b"c"))
197+
.unwrap(),
198+
vec![],
199+
);
146200
}
147201

148202
#[test]

‎mini-lsm-starter/src/mem_table.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ impl MemTable {
8383
lower: Bound<&[u8]>,
8484
upper: Bound<&[u8]>,
8585
) -> MemTableIterator {
86+
// This function is only used in week 1 tests, so during the week 3 key-ts refactor, you do
87+
// not need to consider the bound exclude/include logic. Simply provide `DEFAULT_TS` as the
88+
// timestamp for the key-ts pair.
8689
self.scan(lower, upper)
8790
}
8891

‎mini-lsm/src/mem_table.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ impl MemTable {
9393
lower: Bound<&[u8]>,
9494
upper: Bound<&[u8]>,
9595
) -> MemTableIterator {
96+
// This function is only used in week 1 tests, so during the week 3 key-ts refactor, you do
97+
// not need to consider the bound exclude/include logic. Simply provide `DEFAULT_TS` as the
98+
// timestamp for the key-ts pair.
9699
self.scan(lower, upper)
97100
}
98101

‎mini-lsm/src/tests/harness.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,10 @@ where
127127
);
128128
iter.next().unwrap();
129129
}
130-
assert!(!iter.is_valid());
130+
assert!(
131+
!iter.is_valid(),
132+
"iterator should not be valid at the end of the check"
133+
);
131134
}
132135

133136
pub fn check_iter_result_by_key_and_ts<I>(iter: &mut I, expected: Vec<((Bytes, u64), Bytes)>)

0 commit comments

Comments
 (0)