Skip to content
This repository has been archived by the owner on Aug 4, 2024. It is now read-only.

Commit

Permalink
Enable clippy in ci (#55)
Browse files Browse the repository at this point in the history
* Improve build

* Fix ci

* Enable clippy workflow

* Fix conflict

* Install protoc before clippy

* Fix lints

* Fix lints

* fmt

* Fix lints
  • Loading branch information
lewiszlw authored Oct 28, 2023
1 parent 0436b3e commit fbedc37
Show file tree
Hide file tree
Showing 16 changed files with 65 additions and 79 deletions.
19 changes: 7 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,21 @@ env:
CARGO_REGISTRIES_MY_REGISTRY_INDEX: https://github.com/rust-lang/crates.io-index

jobs:
fmt:
runs-on: ubuntu-20.04
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
with:
profile: minimal
toolchain: nightly
components: rustfmt, clippy
- name: Check code format
uses: actions-rs/cargo@v1
with:
command: fmt
args: --all -- --check
# TODO follow up pr to fix clippy lints
# - name: Run cargo clippy
# uses: actions-rs/cargo@v1
# with:
# command: clippy
# args: -- -D warnings
- name: Install Protoc
uses: arduino/setup-protoc@v2
- run: cargo clippy --all-targets --all-features -- -D warnings


build:
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async fn main() -> ConnectionResult<()> {
.batch_get(keys)
.await?
.into_iter()
.map(|value| decode(value))
.map(decode)
.collect_vec()
)
}
Expand Down
6 changes: 3 additions & 3 deletions src/kernel/lsm/compactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ mod tests {
|| !level_slice[6].is_empty()
);

for (level, slice) in level_slice.into_iter().enumerate() {
for (level, slice) in level_slice.iter().enumerate() {
if !slice.is_empty() && level != LEVEL_0 {
let mut tmp_scope: Option<&Scope> = None;

Expand All @@ -402,8 +402,8 @@ mod tests {
assert_eq!(kv_store.len().await?, times);

let start = Instant::now();
for i in 0..times {
assert_eq!(kv_store.get(&vec_kv[i].0).await?, Some(vec_kv[i].1.clone()));
for kv in vec_kv.iter().take(times) {
assert_eq!(kv_store.get(&kv.0).await?, Some(kv.1.clone()));
}
println!("[get_for][Time: {:?}]", start.elapsed());
kv_store.flush().await?;
Expand Down
6 changes: 3 additions & 3 deletions src/kernel/lsm/iterator/level_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ mod tests {
ver_status
.loader()
.create(2, slice_2.to_vec(), 1, TableType::Skip)?;
let fusion_meta = TableMeta::fusion(&vec![meta_1, meta_2]);
let fusion_meta = TableMeta::fusion(&[meta_1, meta_2]);

let vec_edit = vec![
// Tips: 由于level 0只是用于测试seek是否发生错误,因此可以忽略此处重复使用
Expand All @@ -156,8 +156,8 @@ mod tests {
let version = ver_status.current().await;

let mut iterator = LevelIter::new(&version, 1)?;
for i in 0..times {
assert_eq!(iterator.try_next()?.unwrap(), vec_data[i]);
for kv in vec_data.iter().take(times) {
assert_eq!(iterator.try_next()?.unwrap(), kv.clone());
}

assert_eq!(
Expand Down
5 changes: 2 additions & 3 deletions src/kernel/lsm/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,7 @@ mod tests {
"and my third",
];

let mut dst = Vec::new();
dst.resize(1024, 0u8);
let mut dst = vec![0; 1024];

{
let mut lw = LogWriter::new(Cursor::new(dst.as_mut_slice()));
Expand Down Expand Up @@ -355,7 +354,7 @@ mod tests {
loop {
let r = lr.read(&mut dst);

if !r.is_ok() {
if r.is_err() {
panic!("{}", r.unwrap_err());
} else if r.unwrap() == 0 {
break;
Expand Down
12 changes: 6 additions & 6 deletions src/kernel/lsm/mem_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ impl MemTable {
._immut
.as_ref()
.map(|mem_map| Self::_range_scan(mem_map, min, max, option_seq))
.unwrap_or(vec![])
.unwrap_or_default()
.into_iter()
.chain(Self::_range_scan(&inner._mem, min, max, option_seq))
.rev()
Expand Down Expand Up @@ -451,26 +451,26 @@ mod tests {
let old_seq_id = Sequence::create();

assert_eq!(
mem_table.find(&vec![b'k']),
mem_table.find(&[b'k']),
Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'1']))))
);

let _ = mem_table.insert_data(data_2)?;

assert_eq!(
mem_table.find(&vec![b'k']),
mem_table.find(&[b'k']),
Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'2']))))
);

assert_eq!(
mem_table.find_with_sequence_id(&vec![b'k'], old_seq_id),
mem_table.find_with_sequence_id(&[b'k'], old_seq_id),
Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'1']))))
);

let new_seq_id = Sequence::create();

assert_eq!(
mem_table.find_with_sequence_id(&vec![b'k'], new_seq_id),
mem_table.find_with_sequence_id(&[b'k'], new_seq_id),
Some((Bytes::from(vec![b'k']), Some(Bytes::from(vec![b'2']))))
);

Expand Down Expand Up @@ -632,7 +632,7 @@ mod tests {
assert_eq!(iter.try_next()?, None);

assert_eq!(
iter.seek(Seek::Backward(&vec![b'3']))?,
iter.seek(Seek::Backward(&[b'3']))?,
Some((key_4_2.key.clone(), None))
);

Expand Down
41 changes: 18 additions & 23 deletions src/kernel/lsm/mvcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct Transaction {

impl Transaction {
fn write_buf_or_init(&mut self) -> &mut SkipMap<Bytes, Option<Bytes>> {
self.write_buf.get_or_insert_with(|| SkipMap::new())
self.write_buf.get_or_insert_with(SkipMap::new)
}

/// 通过Key获取对应的Value
Expand Down Expand Up @@ -118,17 +118,16 @@ impl Transaction {
}

fn _mem_range(&self, min: Bound<&[u8]>, max: Bound<&[u8]>) -> Option<MapIter> {
if let Some(buf) = &self.write_buf {
#[allow(clippy::option_map_or_none)]
self.write_buf.as_ref().map_or(None, |buf| {
Some(
buf.range(
min.map(Bytes::copy_from_slice).as_ref(),
max.map(Bytes::copy_from_slice).as_ref(),
)
.map(|(key, value)| (key.clone(), value.clone())),
)
} else {
None
}
})
}

fn mem_table(&self) -> &MemTable {
Expand Down Expand Up @@ -325,38 +324,34 @@ mod tests {
}

// 模拟数据分布在MemTable以及SSTable中
for i in 0..50 {
kv_store
.set(vec_kv[i].0.clone(), vec_kv[i].1.clone())
.await?;
for kv in vec_kv.iter().take(50) {
kv_store.set(kv.0.clone(), kv.1.clone()).await?;
}

kv_store.flush().await?;

for i in 50..100 {
kv_store
.set(vec_kv[i].0.clone(), vec_kv[i].1.clone())
.await?;
for kv in vec_kv.iter().take(100).skip(50) {
kv_store.set(kv.0.clone(), kv.1.clone()).await?;
}

let mut tx_1 = kv_store.new_transaction().await;

for i in 100..times {
tx_1.set(vec_kv[i].0.clone(), vec_kv[i].1.clone());
for kv in vec_kv.iter().take(times).skip(100) {
tx_1.set(kv.0.clone(), kv.1.clone());
}

tx_1.remove(&vec_kv[times - 1].0)?;

// 事务在提交前事务可以读取到自身以及Store已写入的数据
for i in 0..times - 1 {
assert_eq!(tx_1.get(&vec_kv[i].0)?, Some(vec_kv[i].1.clone()));
for kv in vec_kv.iter().take(times - 1) {
assert_eq!(tx_1.get(&kv.0)?, Some(kv.1.clone()));
}

assert_eq!(tx_1.get(&vec_kv[times - 1].0)?, None);

// 事务在提交前Store不应该读取到事务中的数据
for i in 100..times {
assert_eq!(kv_store.get(&vec_kv[i].0).await?, None);
for kv in vec_kv.iter().take(times).skip(100) {
assert_eq!(kv_store.get(&kv.0).await?, None);
}

let vec_test = vec_kv[25..]
Expand All @@ -368,17 +363,17 @@ mod tests {
let mut iter = tx_1.iter(Bound::Included(&vec_kv[25].0), Bound::Unbounded)?;

// -1是因为最后一个元素在之前tx中删除了,因此为None
for i in 0..vec_test.len() - 1 {
for kv in vec_test.iter().take(vec_test.len() - 1) {
// 元素太多,因此这里就单个对比,否则会导致报错时日志过多
assert_eq!(iter.try_next()?.unwrap(), vec_test[i]);
assert_eq!(iter.try_next()?.unwrap(), kv.clone());
}

drop(iter);

tx_1.commit().await?;

for i in 0..times - 1 {
assert_eq!(kv_store.get(&vec_kv[i].0).await?, Some(vec_kv[i].1.clone()));
for kv in vec_kv.iter().take(times - 1) {
assert_eq!(kv_store.get(&kv.0).await?, Some(kv.1.clone()));
}

Ok(())
Expand Down
8 changes: 4 additions & 4 deletions src/kernel/lsm/table/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,9 @@ mod tests {
ss_table_loaded.query(&repeat_data.0)?,
Some(repeat_data.clone())
);
for i in 1..times {
for kv in vec_data.iter().take(times).skip(1) {
assert_eq!(
ss_table_loaded.query(&vec_data[i].0)?.unwrap().1,
ss_table_loaded.query(&kv.0)?.unwrap().1,
Some(value.clone())
)
}
Expand All @@ -228,9 +228,9 @@ mod tests {
ss_table_backup.query(&repeat_data.0)?,
Some(repeat_data.clone())
);
for i in 1..times {
for kv in vec_data.iter().take(times).skip(1) {
assert_eq!(
ss_table_backup.query(&vec_data[i].0)?.unwrap().1,
ss_table_backup.query(&kv.0)?.unwrap().1,
Some(value.clone())
)
}
Expand Down
10 changes: 5 additions & 5 deletions src/kernel/lsm/table/ss_table/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,8 +735,8 @@ mod tests {

let mut cache = LruCache::new(5)?;

for i in 0..times {
let key = &vec_data[i].0;
for kv in vec_data.iter().take(times) {
let key = &kv.0;
let data_block = cache.get_or_insert(index_block.find_with_upper(key), |index| {
let &Index { offset, len } = index;
let target_block = Block::<Value>::decode(
Expand Down Expand Up @@ -779,8 +779,8 @@ mod tests {
let rand_max_idx: usize = rng.gen_range(2..all_item.len() - 1);
let rand_min_idx: usize = rng.gen_range(0..rand_max_idx - 1);

let min_key = full_key(&block, &block.vec_entry, rand_min_idx);
let max_key = full_key(&block, &block.vec_entry, rand_max_idx);
let min_key = full_key(block, &block.vec_entry, rand_min_idx);
let max_key = full_key(block, &block.vec_entry, rand_max_idx);

let excluded_vec = block.range(Bound::Excluded(&min_key), Bound::Excluded(&max_key));

Expand All @@ -803,7 +803,7 @@ mod tests {

fn full_key<T: BlockItem + Eq + Debug>(
block: &Block<T>,
all_entry: &Vec<(usize, Entry<T>)>,
all_entry: &[(usize, Entry<T>)],
index: usize,
) -> Vec<u8> {
let entry = &all_entry[index].1;
Expand Down
8 changes: 4 additions & 4 deletions src/kernel/lsm/table/ss_table/block_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,15 @@ mod tests {
);

assert_eq!(
iterator.seek(Seek::Backward(&vec![b'2']))?,
iterator.seek(Seek::Backward(&[b'2']))?,
Some((
Bytes::from(vec![b'2']),
Value::from(Some(Bytes::from(vec![b'0'])))
))
);

assert_eq!(
iterator.seek(Seek::Backward(&vec![b'3']))?,
iterator.seek(Seek::Backward(&[b'3']))?,
Some((Bytes::from(vec![b'4']), Value::from(None)))
);

Expand All @@ -204,8 +204,8 @@ mod tests {
tokio_test::block_on(async move {
let mut iterator = BlockIter::new(&block);

for i in 0..times {
assert_eq!(iterator.try_next()?.unwrap(), vec_data[i]);
for kv in vec_data.iter().take(times) {
assert_eq!(iterator.try_next()?.unwrap(), kv.clone());
}

for i in (0..times - 1).rev() {
Expand Down
6 changes: 3 additions & 3 deletions src/kernel/lsm/table/ss_table/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl<'a> SSTableIter<'a> {
.cache
.get_or_insert((ss_table.gen(), Some(index)), |(_, index)| {
let index = (*index).ok_or_else(|| KernelError::DataEmpty)?;
Ok(ss_table.data_block(index)?)
ss_table.data_block(index)
})
.map(|block_type| match block_type {
BlockType::Data(data_block) => Some(data_block),
Expand Down Expand Up @@ -154,8 +154,8 @@ mod tests {

let mut iterator = SSTableIter::new(&ss_table)?;

for i in 0..times {
assert_eq!(iterator.try_next()?.unwrap(), vec_data[i]);
for kv in vec_data.iter().take(times) {
assert_eq!(iterator.try_next()?.unwrap(), kv.clone());
}

for i in (0..times - 1).rev() {
Expand Down
16 changes: 5 additions & 11 deletions src/kernel/lsm/table/ss_table/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl Table for SSTable {
(self.gen(), Some(index_block.find_with_upper(key))),
|(_, index)| {
let index = (*index).ok_or_else(|| KernelError::DataEmpty)?;
Ok(Self::data_block(self, index)?)
Self::data_block(self, index)
},
)? {
if let (value, true) = data_block.find(key) {
Expand Down Expand Up @@ -291,20 +291,14 @@ mod tests {

let ss_table = sst_loader.get(1).unwrap();

for i in 0..times {
assert_eq!(
ss_table.query(&vec_data[i].0)?.unwrap().1,
Some(value.clone())
)
for kv in vec_data.iter().take(times) {
assert_eq!(ss_table.query(&kv.0)?.unwrap().1, Some(value.clone()))
}
let cache = ShardingLruCache::new(config.table_cache_size, 16, RandomState::default())?;
let ss_table =
SSTable::load_from_file(sst_factory.reader(1, IoType::Direct)?, Arc::new(cache))?;
for i in 0..times {
assert_eq!(
ss_table.query(&vec_data[i].0)?.unwrap().1,
Some(value.clone())
)
for kv in vec_data.iter().take(times) {
assert_eq!(ss_table.query(&kv.0)?.unwrap().1, Some(value.clone()))
}

Ok(())
Expand Down
1 change: 1 addition & 0 deletions src/kernel/lsm/version/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ impl Version {
}
}

#[allow(clippy::filter_map_bool_then)]
self.level_slice
.iter()
.enumerate()
Expand Down
Loading

0 comments on commit fbedc37

Please sign in to comment.