lightning: finer granularity for tidb backend finished bytes #39318

lance6716 · 2022-11-22T10:38:22Z

Signed-off-by: lance6716 lance6716@gmail.com

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

I still use the old Lightning.Status to get the finished bytes and total bytes. For TiDB backend, finished bytes will be counted after write to TiDB, and also will recover from checkpoints

What is changed and how it works?

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

Documentation

Release note

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

None

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot · 2022-11-22T10:38:24Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
okJiang

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

lance6716 · 2022-11-22T10:38:44Z

/cc @D3Hunter @okJiang

I'm working on it

Signed-off-by: lance6716 <lance6716@gmail.com>

br/pkg/lightning/lightning.go

lance6716 · 2022-11-23T02:16:05Z

/run-integration-br-tests

Signed-off-by: lance6716 <lance6716@gmail.com>

…ver-metrics

lance6716 · 2022-11-23T02:43:44Z

/run-integration-br-tests

D3Hunter · 2022-11-23T02:54:58Z

br/pkg/lightning/restore/table_restore.go

@@ -406,6 +406,11 @@ func (tr *TableRestore) restoreEngine(
 		if err != nil {
 			return closedEngine, errors.Trace(err)
 		}
+		if rc.status != nil && rc.status.backend == config.BackendTiDB {
+			for _, chunk := range cp.Chunks {
+				rc.status.FinishedFileSize.Add(chunk.Chunk.EndOffset)


should it be chunk.Chunk.EndOffset-chunk.Key.Offset, in case of strict format where chunk.Key.Offset is not 0

I'm not familiar with strict format, can you give some reference to the behaviour?

I think you want to keep a property that the value of FinishedFileSize.Add should be same no matter it's processed without failure or recovered from checkpoint?

OK I guess the strict mode will split a file into many chunks, the split position is written in Key.Offset

D3Hunter · 2022-11-23T03:07:09Z

br/pkg/lightning/restore/table_restore.go

@@ -475,6 +480,9 @@ func (tr *TableRestore) restoreEngine(

 	// Restore table data
 	for chunkIndex, chunk := range cp.Chunks {
+		if rc.status != nil && rc.status.backend == config.BackendTiDB {
+			rc.status.FinishedFileSize.Add(chunk.Chunk.Offset)


should it be chunk.Chunk.Offset - chunk.Key.Offset to count previous imported data

okJiang · 2022-11-23T03:06:53Z

br/pkg/lightning/restore/table_restore.go

@@ -475,6 +480,9 @@ func (tr *TableRestore) restoreEngine(

 	// Restore table data
 	for chunkIndex, chunk := range cp.Chunks {
+		if rc.status != nil && rc.status.backend == config.BackendTiDB {
+			rc.status.FinishedFileSize.Add(chunk.Chunk.Offset)


seems that not restore completedly here?

here I want to recover FinishedFileSize from checkpoint, the restore process of current run is not started

br/pkg/lightning/restore/table_restore.go

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 · 2022-11-23T07:11:12Z

/run-integration-br-tests

okJiang · 2022-11-23T07:45:47Z

/merge

ti-chi-bot · 2022-11-23T07:45:51Z

This pull request has been accepted and is ready to merge.

Commit hash: 6250609

sre-bot · 2022-11-23T08:28:35Z

TiDB MergeCI notify

✅ Well Done! New fixed [2] after this pr merged.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci/integration-cdc-test	🔴 failed 1, success 38, total 39	20 min	Existing failure
idc-jenkins-ci-tidb/sqllogic-test-2	🔴 failed 6, success 22, total 28	12 min	Existing failure
idc-jenkins-ci-tidb/mybatis-test	🔴 failed 1, success 0, total 1	11 min	Existing failure
idc-jenkins-ci-tidb/integration-ddl-test	🔴 failed 1, success 5, total 6	4 min 33 sec	Existing failure
idc-jenkins-ci-tidb/integration-common-test	✅ all 17 tests passed	18 min	Fixed
idc-jenkins-ci-tidb/common-test	✅ all 11 tests passed	14 min	Fixed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	6 min 12 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	5 min 13 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 57 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

lightning: recover some metrics from checkpoint

14d3567

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 22, 2022

ti-chi-bot requested review from D3Hunter and okJiang November 22, 2022 10:38

lance6716 added 4 commits November 22, 2022 18:57

sync code

29593bd

Signed-off-by: lance6716 <lance6716@gmail.com>

sync code

f4e36dc

Signed-off-by: lance6716 <lance6716@gmail.com>

sync code

d0356ab

Signed-off-by: lance6716 <lance6716@gmail.com>

try impl

e97a4cc

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 23, 2022

lance6716 added 2 commits November 23, 2022 09:59

sync code

c373c08

Signed-off-by: lance6716 <lance6716@gmail.com>

finish

3457792

Signed-off-by: lance6716 <lance6716@gmail.com>

lance6716 commented Nov 23, 2022

View reviewed changes

br/pkg/lightning/lightning.go Outdated Show resolved Hide resolved

Update br/pkg/lightning/lightning.go

912d1a8

lance6716 changed the title ~~[WIP]lightning: recover some metrics from checkpoint~~ lightning: finer granularity for tidb backend finished bytes Nov 23, 2022

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 23, 2022

lance6716 added 2 commits November 23, 2022 10:42

fix unexpected change

7c1de7e

Signed-off-by: lance6716 <lance6716@gmail.com>

Merge branch 'recover-metrics' of github.com:lance6716/tidb into reco…

2743ae4

…ver-metrics

D3Hunter reviewed Nov 23, 2022

View reviewed changes

okJiang reviewed Nov 23, 2022

View reviewed changes

consider chunk.Key.Offset

6250609

Signed-off-by: lance6716 <lance6716@gmail.com>

D3Hunter approved these changes Nov 23, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 23, 2022

okJiang approved these changes Nov 23, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 23, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 23, 2022

ti-chi-bot added 2 commits November 23, 2022 15:45

Merge branch 'master' into recover-metrics

fe73373

Merge branch 'master' into recover-metrics

303d3ee

ti-chi-bot merged commit f50113f into pingcap:master Nov 23, 2022

lance6716 mentioned this pull request Nov 23, 2022

lightning(dm): update dependency for lightning pingcap/tiflow#7698

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lightning: finer granularity for tidb backend finished bytes #39318

lightning: finer granularity for tidb backend finished bytes #39318

lance6716 commented Nov 22, 2022 •

edited

Loading

ti-chi-bot commented Nov 22, 2022 •

edited

Loading

lance6716 commented Nov 22, 2022

lance6716 commented Nov 23, 2022

lance6716 commented Nov 23, 2022

D3Hunter Nov 23, 2022

lance6716 Nov 23, 2022

lance6716 Nov 23, 2022 •

edited

Loading

D3Hunter Nov 23, 2022

okJiang Nov 23, 2022

lance6716 Nov 23, 2022

lance6716 commented Nov 23, 2022

okJiang commented Nov 23, 2022

ti-chi-bot commented Nov 23, 2022

sre-bot commented Nov 23, 2022

lightning: finer granularity for tidb backend finished bytes #39318

lightning: finer granularity for tidb backend finished bytes #39318

Conversation

lance6716 commented Nov 22, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 22, 2022 • edited Loading

lance6716 commented Nov 22, 2022

lance6716 commented Nov 23, 2022

lance6716 commented Nov 23, 2022

D3Hunter Nov 23, 2022

Choose a reason for hiding this comment

lance6716 Nov 23, 2022

Choose a reason for hiding this comment

lance6716 Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

D3Hunter Nov 23, 2022

Choose a reason for hiding this comment

okJiang Nov 23, 2022

Choose a reason for hiding this comment

lance6716 Nov 23, 2022

Choose a reason for hiding this comment

lance6716 commented Nov 23, 2022

okJiang commented Nov 23, 2022

ti-chi-bot commented Nov 23, 2022

sre-bot commented Nov 23, 2022

TiDB MergeCI notify

lance6716 commented Nov 22, 2022 •

edited

Loading

ti-chi-bot commented Nov 22, 2022 •

edited

Loading

lance6716 Nov 23, 2022 •

edited

Loading