tikv icon indicating copy to clipboard operation
tikv copied to clipboard

backup: Migrate backup component from lazy_static to LazyLock

Open Lymah123 opened this issue 2 months ago • 19 comments

What is changed and how it works?

Issue Number: Close #18927

This PR refactors the backup component to replace the lazy_static! macro with std::sync::LazyLock .TiKV uses nightly Rust (1.87-nightly), and the LazyLock API we rely on primarily LazyLock::new is fully stable and available. Using LazyLock removes the need for an external dependency and provides a cleaner, modern initialization pattern.

What's Changed:

-  Replace lazy_static! macro with std::sync::LazyLock
-  Convert static metrics into accessor functions backed by LazyLock
-  Update all metric usages in endpoint.rs, writer.rs, errors.rs, and metrics.rs to use the new accessor functions
- Remove lazy_static dependency from Cargo.toml

Related changes

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

Check List

Tests

  • [x] 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

Lymah123 avatar Oct 03 '25 01:10 Lymah123

Hi @Lymah123. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

ti-chi-bot[bot] avatar Oct 03 '25 01:10 ti-chi-bot[bot]

Welcome @Lymah123!

It looks like this is your first PR to tikv/tikv 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to tikv/tikv. :smiley:

ti-chi-bot[bot] avatar Oct 03 '25 01:10 ti-chi-bot[bot]

@v01dstar, I look forward to your feedback on this PR. Thanks.

Lymah123 avatar Oct 03 '25 01:10 Lymah123

@v01dstar, I look forward to your feedback on this PR. Thanks.

Please fix the CI first

v01dstar avatar Oct 04 '25 06:10 v01dstar

@v01dstar, I look forward to your feedback on this PR. Thanks.

Please fix the CI first

Fixed @v01dstar

Lymah123 avatar Oct 06 '25 00:10 Lymah123

Hello @v01dstar , I have made requested changes.

Lymah123 avatar Oct 13 '25 18:10 Lymah123

Please fix the tests

v01dstar avatar Nov 18 '25 21:11 v01dstar

Please fix the tests

OK

Lymah123 avatar Nov 20 '25 10:11 Lymah123

Please fix the tests

Fixed @v01dstar

Lymah123 avatar Nov 20 '25 14:11 Lymah123

/ok-to-test

v01dstar avatar Nov 25 '25 19:11 v01dstar

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: v01dstar

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

The pull request process is described here

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[bot] avatar Nov 25 '25 19:11 ti-chi-bot[bot]

[LGTM Timeline notifier]

Timeline:

  • 2025-11-25 19:38:17.538502999 +0000 UTC m=+645261.187697446: :ballot_box_with_check: agreed by v01dstar.

ti-chi-bot[bot] avatar Nov 25 '25 19:11 ti-chi-bot[bot]

Hi @v01dstar , is the PR going to get merged?

Lymah123 avatar Dec 04 '25 19:12 Lymah123

Hi @v01dstar , is the PR going to get merged?

Please fix the CI first

v01dstar avatar Dec 04 '25 19:12 v01dstar

Hi @v01dstar , I tested locally , all the test cases passed. Screenshot (59)

The failing test (test_raftstore_new_server_cluster) is unrelated to my changes.

My PR only modified the redact_option_key helper function at line 1114 to fix a compilation error in logging code. The function is used at line 1110 to safely log Option<Key> values.

The test failure is:

  • A flaky integration test (passed 2 times, failed once)
  • A Raft consensus issue peer is not leader for region 1015
  • Related to snapshot synchronization, not backup endpoint logging

My changes:

  • Fixed a compilation error in redact_option_key
  • Code compiles successfully with no errors
  • Changes are isolated to logging utilities only

The test should pass on retry. My changes don't affect Raft operations

What do you think I can do to passed the failing test?

Lymah123 avatar Dec 05 '25 22:12 Lymah123

/test all

v01dstar avatar Dec 06 '25 06:12 v01dstar

The test cases have passed @v01dstar

Lymah123 avatar Dec 08 '25 21:12 Lymah123

Please update your PR title and description to reflect the use of LazyLock instead of OnceLock. Also update the "Release note" section of the PR description to be "None" since it is a refactoring. Meanwhile, I will ping another reviewer to take a look.

v01dstar avatar Dec 08 '25 21:12 v01dstar

Please update your PR title and description to reflect the use of LazyLock instead of OnceLock. Also update the "Release note" section of the PR description to be "None" since it is a refactoring. Meanwhile, I will ping another reviewer to take a look.

Done @v01dstar!

Lymah123 avatar Dec 09 '25 15:12 Lymah123