*: add global sort related metric and fix cancel add index #47485

wjhuang2016 · 2023-10-09T09:41:35Z

What problem does this PR solve?

Issue Number: ref #45719 and close #47715 #47534

Problem Summary:
Add global sort related metric.

What is changed and how it works?

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.

None

Signed-off-by: wjhuang2016 <[email protected]>

codecov · 2023-10-09T09:56:54Z

Codecov Report

Merging #47485 (6709be1) into master (4b0b47c) will increase coverage by 0.8911%.
Report is 11 commits behind head on master.
The diff coverage is 98.7804%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #47485        +/-   ##
================================================
+ Coverage   71.9282%   72.8194%   +0.8911%     
================================================
  Files          1396       1421        +25     
  Lines        404781     411938      +7157     
================================================
+ Hits         291152     299971      +8819     
+ Misses        94027      93126       -901     
+ Partials      19602      18841       -761

Flag	Coverage Δ
integration	`41.3468% <67.0731%> (?)`
unit	`71.9481% <98.7804%> (+0.0198%)`	⬆️

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

Components	Coverage Δ
dumpling	`53.9913% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.6947% <100.0000%> (-4.2354%)`	⬇️

Signed-off-by: wjhuang2016 <[email protected]>

lance6716 · 2023-10-16T11:49:08Z

br/pkg/lightning/backend/external/engine.go

 	}
 	if iter.Error() != nil {
 		return nil, errors.Trace(iter.Error())
 	}

+	metrics.GlobalSortReadFromCloudStorageRate.WithLabelValues("read_and_sort").Observe(float64(totalSize) / 1024.0 / 1024.0 / time.Since(startTs).Seconds())


for hot path we should cache the result of WithLabelValues to avoid the inner lookup.

lance6716 · 2023-10-16T11:50:02Z

br/pkg/lightning/backend/external/iter.go

@@ -229,6 +229,8 @@ func (i *mergeIter[T, R]) next() bool {
 				// currently only one reader will become hotspot
 				if cnt > (checkHotspotPeriod / 2) {
 					i.lastHotspotIdx = idx
+
+					logutil.BgLogger().Info("new hotspot reader", zap.Any("idx", i.lastHotspotIdx), zap.Any("cnt", cnt))


I remember the comment of Any says it's a bit slower, we can use zap.Int64 instead

It's for debug, I forgot to remove it

Signed-off-by: wjhuang2016 <[email protected]>

ti-chi-bot · 2023-10-17T02:34:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lance6716, ywqzzy

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:

~~OWNERS~~ [lance6716]
~~br/pkg/lightning/OWNERS~~ [lance6716]

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

ywqzzy · 2023-10-17T02:34:30Z

/retest

ti-chi-bot · 2023-10-17T02:34:32Z

[LGTM Timeline notifier]

Timeline:

2023-10-16 11:52:02.870331433 +0000 UTC m=+1657920.457441579: ☑️ agreed by lance6716.
2023-10-17 02:34:31.24161649 +0000 UTC m=+1710868.828726634: ☑️ agreed by ywqzzy.

D3Hunter · 2023-10-17T07:35:53Z

/cherry-pick release-7.5

ti-chi-bot · 2023-10-17T07:36:38Z

@D3Hunter: new pull request created to branch release-7.5: #47710.

In response to this:

/cherry-pick release-7.5

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 ti-community-infra/tichi repository.

ref pingcap#45719

…47710) ref #45719, close #47715

ref pingcap#45719

temp

92912ef

Signed-off-by: wjhuang2016 <[email protected]>

ti-chi-bot bot added do-not-merge/invalid-title do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 9, 2023

wjhuang2016 added 2 commits October 10, 2023 18:45

try

a2fae1d

Signed-off-by: wjhuang2016 <[email protected]>

test

8afb6b3

Signed-off-by: wjhuang2016 <[email protected]>

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 10, 2023

wjhuang2016 added 7 commits October 10, 2023 20:10

fix

9a8c2b8

Signed-off-by: wjhuang2016 <[email protected]>

try

0d94923

Signed-off-by: wjhuang2016 <[email protected]>

debug

eac3a23

Signed-off-by: wjhuang2016 <[email protected]>

fix

e9040fd

Signed-off-by: wjhuang2016 <[email protected]>

Merge branch 'master' of https://github.com/pingcap/tidb into add_metric

8d1908f

try

bfa8343

Signed-off-by: wjhuang2016 <[email protected]>

fix

5cdf73a

Signed-off-by: wjhuang2016 <[email protected]>

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/needs-linked-issue labels Oct 16, 2023

wjhuang2016 changed the title ~~WIP~~ *: add global sort related metric Oct 16, 2023

ti-chi-bot bot removed do-not-merge/invalid-title do-not-merge/needs-tests-checked labels Oct 16, 2023

wjhuang2016 added 6 commits October 16, 2023 17:22

merge

0163f95

Signed-off-by: wjhuang2016 <[email protected]>

done

71fb3b1

Signed-off-by: wjhuang2016 <[email protected]>

fix bazel

7dfbdac

Signed-off-by: wjhuang2016 <[email protected]>

refine

b5b27e3

Signed-off-by: wjhuang2016 <[email protected]>

refine

7d0be36

Signed-off-by: wjhuang2016 <[email protected]>

refine

20c0878

Signed-off-by: wjhuang2016 <[email protected]>

comment

ba04a2d

Signed-off-by: wjhuang2016 <[email protected]>

lance6716 approved these changes Oct 16, 2023

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Oct 16, 2023

fix

6709be1

Signed-off-by: wjhuang2016 <[email protected]>

ywqzzy approved these changes Oct 17, 2023

View reviewed changes

ti-chi-bot bot added the lgtm label Oct 17, 2023

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 17, 2023

ti-chi-bot bot merged commit 5ac13d9 into pingcap:master Oct 17, 2023
4 checks passed

D3Hunter mentioned this pull request Oct 17, 2023

importinto: add more logs to external backend, disable parallel sort #47680

Merged

13 tasks

ti-chi-bot mentioned this pull request Oct 17, 2023

*: add global sort related metric and fix cancel add index (#47485) #47710

Merged

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Oct 17, 2023

*: add global sort related metric (pingcap#47485)

f538e0e

ref pingcap#45719

wjhuang2016 changed the title *: add global sort related metric *: add global sort related metric and fix cancel add index Oct 17, 2023

ti-chi-bot bot pushed a commit that referenced this pull request Oct 17, 2023

*: add global sort related metric and fix cancel add index (#47485) (#…

5c6b0ef

…47710) ref #45719, close #47715

ywqzzy mentioned this pull request Oct 25, 2023

add index job status is always running after inject one of tikv network partition and cannot be canceled which lead to next task cannot start #47534

Closed

wuhuizuo pushed a commit to wuhuizuo/tidb that referenced this pull request Apr 2, 2024

*: add global sort related metric (pingcap#47485)

6b7495e

ref pingcap#45719

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add global sort related metric and fix cancel add index #47485

*: add global sort related metric and fix cancel add index #47485

wjhuang2016 commented Oct 9, 2023 •

edited by jebter

Loading

codecov bot commented Oct 9, 2023 •

edited

Loading

lance6716 Oct 16, 2023

lance6716 Oct 16, 2023

wjhuang2016 Oct 16, 2023

ti-chi-bot bot commented Oct 17, 2023

ywqzzy commented Oct 17, 2023

ti-chi-bot bot commented Oct 17, 2023

D3Hunter commented Oct 17, 2023

ti-chi-bot commented Oct 17, 2023

*: add global sort related metric and fix cancel add index #47485

*: add global sort related metric and fix cancel add index #47485

Conversation

wjhuang2016 commented Oct 9, 2023 • edited by jebter Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

codecov bot commented Oct 9, 2023 • edited Loading

Codecov Report

lance6716 Oct 16, 2023

Choose a reason for hiding this comment

lance6716 Oct 16, 2023

Choose a reason for hiding this comment

wjhuang2016 Oct 16, 2023

Choose a reason for hiding this comment

ti-chi-bot bot commented Oct 17, 2023

ywqzzy commented Oct 17, 2023

ti-chi-bot bot commented Oct 17, 2023

[LGTM Timeline notifier]

D3Hunter commented Oct 17, 2023

ti-chi-bot commented Oct 17, 2023

wjhuang2016 commented Oct 9, 2023 •

edited by jebter

Loading

codecov bot commented Oct 9, 2023 •

edited

Loading