br: improve summary and progress visualization of br #56612

Leavrth · 2024-10-12T08:15:05Z

What problem does this PR solve?

Issue Number: close #56493

Problem Summary:
See the issue.

What changed and how does it work?

add more details in the summary. And split the progress of snapshot restore

Detail BR log in /tmp/br.log.2024-10-12T15.39.51+0800 
Split&Scatter Region <----------------------------------------------------------------------------------------------------> 100.00%
Download&Ingest SST <-----------------------------------------------------------------------------------------------------> 100.00%
Restore Pipeline <--------------------------------------------------------------------------------------------------------> 100.00%
[2024/10/12 15:47:55.497 +08:00] [INFO] [collector.go:77] ["Full Restore success summary"] [total-ranges=863] [ranges-succeed=863] [ranges-failed=0] [merge-ranges=7.546971ms] [split-region=343.594072ms] [restore-files=5m3.576621503s] [default-CF-files=278] [write-CF-files=558] [split-keys=386] [total-take=6m35.304887635s] [total-kv=871760372] [total-kv-size=143.8GB] [average-speed=363.8MB/s] [restore-data-size(after-compressed)=62.07GB] [Size=62072785167] [BackupTS=452625823236882437] [RestoreTS=453237563041775617]

merge-ranges: the time consumption of ranges merge.
split-region: the time consumption of regions split&scatter.
restore-files: the time consumption of SST files download&ingest.
default-CF-files: the number of default CF files.
write-CF-files: the number of write CF files.
split-keys: the number of keys to split regions.

Detail BR log in /tmp/br.log.2024-10-17T10.25.13+0800 
Full Backup <------------------------------------------------------------------------------------------------------------------------------> 100.00%
Checksum <---------------------------------------------------------------------------------------------------------------------------------> 100.00%
[2024/10/17 10:31:38.602 +08:00] [INFO] [collector.go:77] ["Full Backup success summary"] [total-ranges=1813] [ranges-succeed=1813] [ranges-failed=0] [backup-fast-checksum=17.400535ms] [backup-checksum=1m26.440733343s] [backup-total-ranges=157] [backup-total-regions=510] [write-CF-files=1188] [default-CF-files=625] [total-take=6m25.007059103s] [total-kv=871788040] [total-kv-size=143.8GB] [average-speed=373.5MB/s] [backup-data-size(after-compressed)=62.07GB] [Size=62073954324] [BackupTS=453281586797346817]

backup-total-regions: the approximate number of regions backed up.
default-CF-files: the number of default CF files.
write-CF-files: the number of write CF files.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

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

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Improve summary and progress visualization of BR

Signed-off-by: Jianjun Liao <[email protected]>

tiprow · 2024-10-12T08:15:24Z

Hi @Leavrth. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

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.

codecov · 2024-10-12T08:28:20Z

Codecov Report

Attention: Patch coverage is 84.14634% with 26 lines in your changes missing coverage. Please review.

Project coverage is 74.9134%. Comparing base (36e8e34) to head (2191949).
Report is 254 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #56612        +/-   ##
================================================
+ Coverage   73.0285%   74.9134%   +1.8849%     
================================================
  Files          1684       1702        +18     
  Lines        465663     471739      +6076     
================================================
+ Hits         340067     353396     +13329     
+ Misses       104691      96664      -8027     
- Partials      20905      21679       +774

Flag	Coverage Δ
integration	`46.5120% <84.1463%> (?)`
unit	`72.6679% <15.8536%> (+0.4561%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.6910% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`61.3691% <84.1463%> (+16.1371%)`	⬆️

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Jianjun Liao <[email protected]>

br/pkg/restore/snap_client/pipeline_items.go

Leavrth · 2024-10-28T02:32:23Z

/retest

tiprow · 2024-10-28T02:32:44Z

@Leavrth: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

br/pkg/restore/snap_client/tikv_sender.go

ti-chi-bot · 2024-11-08T08:50:18Z

PR needs rebase.

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.

Tristan1900

Thanks for the change, it should help a lot of our customers as they are confused why the progress bar stopped at around 70%.

br/pkg/restore/snap_client/client.go

br/pkg/restore/snap_client/pipeline_items.go

Signed-off-by: Jianjun Liao <[email protected]>

BornChanger · 2025-01-07T16:41:42Z

/retest

tiprow · 2025-01-07T16:42:04Z

@BornChanger: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

Signed-off-by: Jianjun Liao <[email protected]>

Leavrth · 2025-01-17T09:21:03Z

/retest

tiprow · 2025-01-17T09:21:32Z

@Leavrth: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

Signed-off-by: Jianjun Liao <[email protected]>

Leavrth · 2025-01-17T10:42:50Z

/ok-to-test

3pointer · 2025-01-21T03:27:19Z

br/pkg/restore/restorer.go

@@ -226,6 +226,9 @@ type MultiTablesRestorer struct {
 	workerPool       *util.WorkerPool
 	fileImporter     BalancedFileImporter
 	checkpointRunner *checkpoint.CheckpointRunner[checkpoint.RestoreKeyType, checkpoint.RestoreValueType]
+
+	fileCount int


need lock here or make it atomic?

The variable fileCount is modified in GoRestore and load in WaitUntilFinish(Notice that fileCount is not used in workerpool or any other goroutines). Actually, the two functions are always appear together like this:

err := restorer.GoRestore() if err != nil { return err } err = restorer.WaitUntilFinish() if err != nil { return err }

ti-chi-bot · 2025-01-22T04:43:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BornChanger, YuJuncen

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:

~~br/OWNERS~~ [BornChanger,YuJuncen]

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 · 2025-01-22T04:43:54Z

[LGTM Timeline notifier]

Timeline:

2025-01-17 06:21:13.376593028 +0000 UTC m=+272344.831639172: ☑️ agreed by BornChanger.
2025-01-22 04:43:53.549623502 +0000 UTC m=+242360.880542906: ☑️ agreed by YuJuncen.

close pingcap#56493

improve visualization of br

48329ac

Signed-off-by: Jianjun Liao <[email protected]>

ti-chi-bot bot added do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 12, 2024

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Oct 12, 2024

Leavrth added 2 commits October 12, 2024 17:14

make bazel

ec04b38

Signed-off-by: Jianjun Liao <[email protected]>

improve visualization of br

5eeb8b4

Signed-off-by: Jianjun Liao <[email protected]>

Leavrth changed the title ~~br: improve visualization of br~~ br: improve summary and progress visualization of br Oct 17, 2024

BornChanger reviewed Oct 17, 2024

View reviewed changes

br/pkg/restore/snap_client/pipeline_items.go Outdated Show resolved Hide resolved

BornChanger reviewed Nov 7, 2024

View reviewed changes

br/pkg/restore/snap_client/tikv_sender.go Outdated Show resolved Hide resolved

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 8, 2024

Tristan1900 reviewed Nov 9, 2024

View reviewed changes

br/pkg/restore/snap_client/client.go Outdated Show resolved Hide resolved

br/pkg/restore/snap_client/pipeline_items.go Outdated Show resolved Hide resolved

Leavrth added 2 commits December 30, 2024 19:26

Merge branch 'master' into visualization_br

a2448db

commit some suggestions

227368e

Signed-off-by: Jianjun Liao <[email protected]>

Merge branch 'master' into visualization_br

2d83c36

BornChanger approved these changes Jan 17, 2025

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 17, 2025

BornChanger added needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. labels Jan 17, 2025

fix unit test

8bc991c

Signed-off-by: Jianjun Liao <[email protected]>

BornChanger removed needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. labels Jan 17, 2025

add restore pipeline duration in summary

2191949

Signed-off-by: Jianjun Liao <[email protected]>

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jan 17, 2025

3pointer reviewed Jan 21, 2025

View reviewed changes

YuJuncen approved these changes Jan 22, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 22, 2025

ti-chi-bot bot merged commit ba79f50 into pingcap:master Jan 22, 2025
29 checks passed

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 10, 2025

Leavrth mentioned this pull request Mar 10, 2025

br: improve visualization of BR pingcap/docs-cn#20025

Merged

16 tasks

zeminzhou pushed a commit to zeminzhou/tidb that referenced this pull request May 6, 2025

br: improve summary and progress visualization of br (pingcap#56612)

3f8520f

close pingcap#56493

kennytm mentioned this pull request May 30, 2025

Progress total on BR-restore v8.5 is overestimated, causing the progress bar to be stuck at a very low value during checksum #61423

Open

Tristan1900 pushed a commit to Tristan1900/tidb that referenced this pull request Jun 6, 2025

br: improve summary and progress visualization of br (pingcap#56612)

289a51e

close pingcap#56493

Leavrth mentioned this pull request Jun 6, 2025

br: cherry pick to hot fix release 8.5 20250114 v8.5.0 second batch | tidb-test=release-8.5.1 #61534

Merged

13 tasks

br: improve summary and progress visualization of br #56612

br: improve summary and progress visualization of br #56612

Uh oh!

Conversation

Leavrth commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Oct 12, 2024

Uh oh!

codecov bot commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Leavrth commented Oct 28, 2024

Uh oh!

tiprow bot commented Oct 28, 2024

Uh oh!

Uh oh!

ti-chi-bot bot commented Nov 8, 2024

Uh oh!

Tristan1900 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BornChanger commented Jan 7, 2025

Uh oh!

tiprow bot commented Jan 7, 2025

Uh oh!

Leavrth commented Jan 17, 2025

Uh oh!

tiprow bot commented Jan 17, 2025

Uh oh!

Leavrth commented Jan 17, 2025

Uh oh!

3pointer Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

Leavrth Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Jan 22, 2025

Uh oh!

ti-chi-bot bot commented Jan 22, 2025

[LGTM Timeline notifier]

Uh oh!

Uh oh!

Uh oh!

Leavrth commented Oct 12, 2024 •

edited

Loading

codecov bot commented Oct 12, 2024 •

edited

Loading