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.

Sign up for GitHub

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

chore: update CI toolchain and clean up code #244

Merged
merged 10 commits into from
Aug 3, 2022
Merged

chore: update CI toolchain and clean up code #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9747bcf
update CI toolchain and improve scan interfaces
tabokie Jul 15, 2022
01e0bd9
restore failpoint
tabokie Jul 15, 2022
59064ba
Merge branch 'master' into cleanup-220715
tabokie Jul 15, 2022
03a8469
Merge branch 'master' into cleanup-220715
tabokie Jul 20, 2022
02ed068
clean up
tabokie Jul 21, 2022
9ec5a77
refactor a bit and add makefile
tabokie Aug 2, 2022
7ab1af2
Merge branch 'master' of https://github.com/tikv/raft-engine into cle…
tabokie Aug 2, 2022
3faee0e
add makefile
tabokie Aug 2, 2022
557d086
refactors
tabokie Aug 3, 2022
f28f309
test fmt works
tabokie Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 19 additions & 20 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly-2022-05-01
toolchain: nightly-2022-07-13
override: true
components: rustfmt, clippy, rust-src
- uses: Swatinem/rust-cache@v1
Expand All @@ -29,24 +29,24 @@ jobs:
if: ${{ matrix.os == 'ubuntu-latest' }}
run: if [[ ! -e ~/.cargo/bin/grcov ]]; then cargo install grcov; fi
- name: Format
run: cargo fmt --all -- --check
run: |
make format
git diff --exit-code
- name: Clippy
run: cargo clippy --all --all-features --all-targets -- -D clippy::all
run: make clippy
- name: Run tests
run: |
cargo test --all --features all_except_failpoints --verbose -- --nocapture
cargo test --test failpoints --all-features --verbose -- --test-threads 1 --nocapture
run: make test
env:
RUST_BACKTRACE: 1
EXTRA_CARGO_ARGS: '--verbose'
- name: Run asan tests
if: ${{ matrix.os == 'ubuntu-latest' }}
run: |
cargo test -Zbuild-std --target x86_64-unknown-linux-gnu --all --features all_except_failpoints --verbose -- --nocapture
cargo test -Zbuild-std --target x86_64-unknown-linux-gnu --test failpoints --all-features --verbose -- --test-threads 1 --nocapture
run: make test
env:
RUST_BACKTRACE: 1
RUSTFLAGS: '-Zsanitizer=address'
RUSTDOCFLAGS: '-Zsanitizer=address'
EXTRA_CARGO_ARGS: '--verbose -Zbuild-std --target x86_64-unknown-linux-gnu'
stable:
runs-on: ${{ matrix.os }}
strategy:
Expand All @@ -66,16 +66,16 @@ jobs:
- uses: Swatinem/rust-cache@v1
with:
sharedKey: ${{ matrix.os }}-stable
- name: Format
run: cargo fmt --all -- --check
- name: Clippy
run: cargo clippy --all --features all_stable --all-targets -- -D clippy::all
run: make clippy
env:
WITH_STABLE_TOOLCHAIN: 'true'
- name: Run tests
run: |
cargo test --all --features all_stable_except_failpoints --verbose -- --nocapture
cargo test --test failpoints --features all_stable --verbose -- --test-threads 1 --nocapture
run: make test
env:
RUST_BACKTRACE: 1
EXTRA_CARGO_ARGS: '--verbose'
WITH_STABLE_TOOLCHAIN: 'true'
coverage:
runs-on: ubuntu-latest
needs: nightly
Expand All @@ -87,7 +87,7 @@ jobs:
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly-2022-05-01
toolchain: nightly-2022-07-13
override: true
components: llvm-tools-preview
- uses: Swatinem/rust-cache@v1
Expand All @@ -97,13 +97,12 @@ jobs:
run: if [[ ! -e ~/.cargo/bin/grcov ]]; then cargo install --locked grcov; fi
- name: Run tests
run: |
cargo test --all --features all_except_failpoints
cargo test --test failpoints --all-features -- --test-threads 1
cargo test --all --features all_stable_except_failpoints
cargo test --test failpoints --features all_stable -- --test-threads 1
make test
env WITH_STABLE_TOOLCHAIN=true make test
env:
RUSTFLAGS: '-Zinstrument-coverage'
LLVM_PROFILE_FILE: '%p-%m.profraw'
EXTRA_CARGO_ARGS: '--verbose'
- name: Run grcov
run: grcov `find . \( -name "*.profraw" \) -print` --binary-path target/debug/deps/ -s . -t lcov --branch --ignore-not-existing --ignore '../**' --ignore '/*' -o coverage.lcov
- name: Upload
Expand Down
34 changes: 34 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Makefile

## Additionaly arguments passed to cargo.
EXTRA_CARGO_ARGS ?=
## Whether to disable nightly-only feature. [true/false]
WITH_STABLE_TOOLCHAIN ?=

.PHONY: format clippy test

all: format clippy test

## Format code in-place using rustfmt.
format:
cargo fmt --all

## Run clippy.
ifeq ($(WITH_STABLE_TOOLCHAIN), true)
clippy:
cargo clippy --all --features all_stable --all-targets -- -D clippy::all
else
clippy:
cargo clippy --all --all-features --all-targets -- -D clippy::all
Copy link
Contributor

@LykxSassinator LykxSassinator Aug 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, don't we need +nightly trait any more when ${WITH_STABLE_TOOLCHAIN} == false ?

I've tested the makefile on Centos:
Linux version 3.10.0-862.14.4.el7.x86_64 ([email protected]) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-28) (GCC) ) #1 SMP Wed Sep 26 15:12:11 UTC 2018

And it returns the following failure:

error[E0554]: `#![feature]` may not be used on the stable release channel
  --> src/lib.rs:16:34
   |
16 | #![cfg_attr(feature = "nightly", feature(test))]
   |                                  ^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
  --> src/lib.rs:17:31

And if I add the +nightly trait, both make test and make clippy worked as expectation without errs, by setting ${WITH_STABLE_TOOLCHAIN} == false .
Also, the feature specification on all_except_failpoints in cargo.toml shows that we need +nightly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to assume user has set the correct default toolchain. But force setting sounds okay too, I'll add it later.

endif

## Run tests.
ifeq ($(WITH_STABLE_TOOLCHAIN), true)
test:
cargo test --all --features all_stable_except_failpoints ${EXTRA_CARGO_ARGS} -- --nocapture
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend to add extra --color always, making the results both on successful and failed tests more readable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you encounter any problem using it? I think the default auto should work good enough.

Copy link
Contributor

@LykxSassinator LykxSassinator Aug 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually，it's work fine in my environment, also on MacOS.

cargo test --test failpoints --features all_stable ${EXTRA_CARGO_ARGS} -- --test-threads 1 --nocapture
else
test:
cargo test --all --features all_except_failpoints ${EXTRA_CARGO_ARGS} -- --nocapture
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

cargo test --test failpoints --all-features ${EXTRA_CARGO_ARGS} -- --test-threads 1 --nocapture
endif
10 changes: 6 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ Contributions are always welcome! Here are a few tips for making a PR:
- Tests are automatically run against the changes, some of them can be run locally:

```
cargo fmt --all -- --check
cargo +nightly clippy --all --all-features --all-targets -- -D clippy::all
cargo +nightly test --all --features all_except_failpoints
cargo +nightly test --test failpoints --all-features -- --test-threads 1
# rustup default nightly
make
# rustup default stable
env WITH_STABLE_TOOLCHAIN=true make
# filter a specific test case
env EXTRA_CARGO_ARGS=<testname> make test
```

- For changes that might induce performance effects, please quote the targeted benchmark results in the PR description. In addition to micro-benchmarks, there is a standalone [stress test tool](https://github.com/tikv/raft-engine/tree/master/stress) which you can use to demonstrate the system performance.
Expand Down
17 changes: 8 additions & 9 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{util::ReadableSize, Result};
const MIN_RECOVERY_READ_BLOCK_SIZE: usize = 512;
const MIN_RECOVERY_THREADS: usize = 1;

#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum RecoveryMode {
AbsoluteConsistency,
Expand Down Expand Up @@ -84,12 +84,11 @@ pub struct Config {
/// Default: None
pub memory_limit: Option<ReadableSize>,

/// Whether to recycle stale logs.
/// If `true`, `purge` operations on logs will firstly put stale
/// files into a list for recycle. It's only available if
/// `format_version` >= `2`.
/// Whether to recycle stale log files.
/// If `true`, logically purged log files will be reserved for recycling.
/// Only available for `format_version` 2 and above.
///
/// Default: false,
/// Default: false
pub enable_log_recycle: bool,
}

Expand Down Expand Up @@ -152,19 +151,19 @@ impl Config {
if self.enable_log_recycle {
if !self.format_version.has_log_signing() {
return Err(box_err!(
"format_version: {:?} is invalid when 'enable_log_recycle' on, setting it to V2",
"format version {} doesn't support log recycle, use 2 or above",
self.format_version
));
}
if self.purge_threshold.0 / self.target_file_size.0 >= std::u32::MAX as u64 {
return Err(box_err!(
"File count exceed UINT32_MAX, calculated by 'purge-threshold / target-file-size'"
"File count exceed u32::MAX, calculated by `purge-threshold / target-file-size`"
));
}
}
#[cfg(not(feature = "swap"))]
if self.memory_limit.is_some() {
warn!("memory-limit will be ignored because swap feature is not enabled");
warn!("memory-limit will be ignored because swap feature is disabled");
}
Ok(())
}
Expand Down
Loading