Skip to content

*: Update rust toolchain to nightly-2025-04-03#18416

Merged
ti-chi-bot[bot] merged 20 commits intotikv:masterfrom
glorv:update-rust
May 15, 2025
Merged

*: Update rust toolchain to nightly-2025-04-03#18416
ti-chi-bot[bot] merged 20 commits intotikv:masterfrom
glorv:update-rust

Conversation

@glorv
Copy link
Contributor

@glorv glorv commented Apr 24, 2025

What is changed and how it works?

Issue Number: Ref #17465

What's Changed:

Following #17605, another attempt to update rust toolchain

Changes:

- Language
  - After https://github.com/rust-lang/rust/pull/134258, we can't manually impl both `ToString` and `fmt::Display`, so this PR add a new trait `ToStringValue` to work around types type produces different result between ToString and Display.
- Clippy
  - `Option::map_or(false, ...)` --> `Option::is_some_and(...)`
  - `Option::map_or(true, ...)` --> `Option::is_none_or(...)`
  - `(a + b - 1 )/ b` --> `a.div_ceil(b)`
  - `io::Error::new(ErrorKind::Other, ...)` --> ` io::Error::other(...)`
  - `Slice::group_by` --> `Slice::chunk_by`
  - `Result::map_err(|e| {...; e})` --> `Result::inspect_err(|e| { ... })`
  - `Map::get(&key).is_{some, none}()` --> `Map::contains_key()`
- Formatter
  - The import order now follows ascii order, e.g. before is "use crate::{a, b, c, A, B, C}", after is "use crate::{A, B, C, a, b, c}". Most changes are due to this.
  - List in rust doc should be properly aligned.
- cargo-deny
  - `vulnerability`, `notice` and `unsound` can't be config in version 2, and `unmaintained` can't be allowed anymore(but support setting `workspace` to allow indirected pkgs). So replacing some unmaintained packages with suggested alternatives.  (See: https://github.com/tikv/tikv/pull/18416)

NOTE: this PR does not update the edition version, we can do this is a separate PR.

Performance Benchmark:

I have benchmarked this PR vs commit c93e28e with benchbot sysbench workloads, the result is:

image
(details: http://perf.pingcap.net/d/iarAjGo7z/benchbot-overview?orgId=1&var-email=All&var-testbed=benchbot-amd64-2xl-sysbench-tps-7759291-1-993&from=1745913985000)

Another benchmark that only ran "oltp_write_only" with the same configurations:

image
(details: http://perf.pingcap.net/d/iarAjGo7z/benchbot-overview?orgId=1&var-email=All&var-testbed=benchbot-amd64-2xl-sysbench-tps-7860707-1-783&from=1745977197000)

So the regression of "oltp_write_only" is likely due to environment issues.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Release note

None

glorv added 2 commits April 24, 2025 16:05
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
@ti-chi-bot ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 24, 2025
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 24, 2025
glorv added 6 commits April 24, 2025 16:19
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
@glorv
Copy link
Contributor Author

glorv commented Apr 25, 2025

glorv added 2 commits April 24, 2025 19:38
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
@hhwyt
Copy link
Contributor

hhwyt commented Apr 25, 2025

Have you compared the build time before and after upgrading Rust?

Copy link
Member

@v01dstar v01dstar left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 25, 2025
@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 25, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Apr 25, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-04-25 05:28:11.673576198 +0000 UTC m=+592635.485366570: ☑️ agreed by v01dstar.
  • 2025-04-25 06:05:17.043927142 +0000 UTC m=+594860.855717523: ☑️ agreed by LykxSassinator.

Comment on lines +169 to 170
#[allow(dead_code)]
fn or_io_error<D: Display>(self, msg: D) -> io::Result<Self::Ok>;
Copy link
Member

Choose a reason for hiding this comment

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

Why not remove it?

glorv added 5 commits April 28, 2025 11:37
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
glorv added 2 commits April 28, 2025 13:45
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
@glorv
Copy link
Contributor Author

glorv commented Apr 28, 2025

/cc @cfzjywxk @zhangjinpeng87 PTAL

@ti-chi-bot ti-chi-bot bot requested review from cfzjywxk and zhangjinpeng87 April 28, 2025 06:19
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Apr 28, 2025

@glorv: GitHub didn't allow me to request PR reviews from the following users: PTAL.

Note that only tikv members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @cfzjywxk @zhangjinpeng87 PTAL

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@glorv
Copy link
Contributor Author

glorv commented Apr 28, 2025

/release

@glorv
Copy link
Contributor Author

glorv commented Apr 28, 2025

Have you compared the build time before and after upgrading Rust?

@hhwyt

Comparing the build to between this PR and master in the CI env:
This PR: 48m47s (https://cd.pingcap.net/blue/organizations/jenkins/build-common/detail/build-common/353771/pipeline/4)
master: 54m52s (https://cd.pingcap.net/blue/organizations/jenkins/build-common/detail/build-common/353769/pipeline/4)

So it's a litter faster, but not much.

@sre-bot
Copy link
Contributor

sre-bot commented Apr 28, 2025

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

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

LGTM

@glorv
Copy link
Contributor Author

glorv commented May 6, 2025

@zhangjinpeng87 PTAL

Copy link
Member

@zhangjinpeng87 zhangjinpeng87 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, Connor1996, hhwyt, LykxSassinator, overvenus, v01dstar, zhangjinpeng87

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the approved label May 14, 2025
@ti-chi-bot ti-chi-bot bot merged commit 079d2a9 into tikv:master May 15, 2025
8 checks passed
@ti-chi-bot ti-chi-bot bot added this to the Pool milestone May 15, 2025
@glorv glorv deleted the update-rust branch May 19, 2025 04:15
@wuhuizuo
Copy link
Contributor

wuhuizuo commented May 20, 2025

There is a building issue caused by it: #18465

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants