importinto: optimize conflicts resolution | tidb-test=release-8.1.2 #61059

D3Hunter · 2025-05-09T11:20:57Z

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

What changed and how does it work?

collect and resolve conflicts in parallel, use subtask concurrency
get/commit KVs in batch
avoid get KV if it's already collected
avoid map query if the savedHandle map is empty
abstract collector and deleter out
delete KVs in a separate group of routines to pipeline get and delete
read conflict KV files in parallel, currently we use 2 routines
fix a bug that we might recreate one file for each conflicted row after the overall row size reach MaxConflictRowFileSize

Check List

Tests

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

schema

create table test.t(
    pk bigint primary key,
    a bigint,
    b bigint,
    c bigint,
    d bigint,
    e bigint,
    pad varchar(1024),
    unique(a),
    unique(b),
    unique(c),
    unique(d),
    index(e)
);

ENV

1 * PD	1 * TiDB	3 * TiKV
8c16g	16c32g	8c32g

for a case with 500000 conflicted rows, all rows are conflicted due to UK. after this PR, conflict resolution speed is about 18518 rows/seconds, ~70 times faster.

+----------------------+----------+----------+
| step                 |  before  |  after   |
+----------------------+----------+----------+
| 0. total             | 00:35:58 | 00:00:53 |
| 1. encode            | 00:00:16 | 00:00:16 |
| 3. ingest            | 00:00:06 | 00:00:06 |
| 4. collect-conflicts | 00:07:45 | 00:00:04 |
| 5. resolve-conflicts | 00:27:46 | 00:00:23 |
| 6. post-process      | 00:00:00 | 00:00:00 |
+----------------------+----------+----------+

for a case with 15000000 conflicted rows, all rows are conflicted due to UK. after this PR, conflict resolution speed is about 20270 rows/seconds.

+----------------------+----------+
| step                 | duration |
+----------------------+----------+
| 0. total             | 00:14:55 |
| 1. encode            | 00:00:40 |
| 3. ingest            | 00:01:46 |
| 4. collect-conflicts | 00:02:27 |
| 5. resolve-conflicts | 00:09:53 |
| 6. post-process      | 00:00:01 |
+----------------------+----------+

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

tiprow · 2025-05-09T11:21:14Z

Hi @D3Hunter. 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.

Details

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 · 2025-05-09T11:58:39Z

Codecov Report

Attention: Patch coverage is 7.28929% with 407 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/release-8.1-gsort-test@60e993c). Learn more about missing BASE report.

Additional details and impacted files

@@                         Coverage Diff                         @@
##             feature/release-8.1-gsort-test     #61059   +/-   ##
===================================================================
  Coverage                                  ?   71.0574%           
===================================================================
  Files                                     ?       1473           
  Lines                                     ?     425816           
  Branches                                  ?          0           
===================================================================
  Hits                                      ?     302574           
  Misses                                    ?     102648           
  Partials                                  ?      20594

Flag	Coverage Δ
unit	`71.0574% <7.2892%> (?)`

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

Components	Coverage Δ
dumpling	`52.9656% <0.0000%> (?)`
parser	`∅ <0.0000%> (?)`
br	`41.6258% <0.0000%> (?)`

🚀 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.

D3Hunter · 2025-05-16T02:58:36Z

/retest

tiprow · 2025-05-16T02:58:58Z

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

Details

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.

joechenrh

Rest LGTM

joechenrh · 2025-05-16T06:07:18Z

pkg/disttask/importinto/collect_conflicts.go

+		c.logger.Info("too many conflict rows from index, skip checking",
+			zap.String("handleSize", units.BytesSize(float64(c.savedHandleSize.Load()))))
+		c.skipSaveHandle = true
+		c.savedHandles = make(map[string]bool)


Not clear the saved handle here and let it merged into e.result? Since we can ensure that the total map size won't exceed the limit.

ti-chi-bot · 2025-05-16T07:13:11Z

@joechenrh: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Rest LGTM

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.

Benjamin2037

LGTM

D3Hunter · 2025-05-16T08:20:46Z

/hold

will fix the comment later

ti-chi-bot · 2025-05-16T09:45:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, GMHDBJD, joechenrh

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:

~~OWNERS~~ [Benjamin2037,GMHDBJD]
~~pkg/disttask/OWNERS~~ [Benjamin2037,GMHDBJD]
~~pkg/lightning/OWNERS~~ [Benjamin2037,GMHDBJD]

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-05-16T09:45:43Z

[LGTM Timeline notifier]

Timeline:

2025-05-16 07:44:10.114354657 +0000 UTC m=+98616.370252054: ☑️ agreed by Benjamin2037.
2025-05-16 09:45:41.695622181 +0000 UTC m=+105907.951519579: ☑️ agreed by GMHDBJD.

D3Hunter · 2025-05-16T10:29:42Z

/unhold

D3Hunter · 2025-05-16T10:31:41Z

/retest

tiprow · 2025-05-16T10:32:04Z

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

Details

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.

D3Hunter added 5 commits April 30, 2025 18:43

change

23e80d5

change

e2b3cf4

change

c215f22

change

9a28742

change

b30580c

D3Hunter added 5 commits May 12, 2025 16:51

change

06b3c55

change

b0567fc

change

e4c2151

change

8145eb8

enlarge concurrency

842b7e3

D3Hunter changed the title ~~[WIP]Parallel resolve conflicts~~ [WIP]Parallel resolve conflicts | tidb-test=release-8.1.2 May 14, 2025

D3Hunter added 13 commits May 14, 2025 21:14

change

d872451

clone

2503408

batch get

333bf87

batch get row key

98b749f

opt for saved handles

d426fee

opt log

60e4b36

fix

5040ce5

change

60d00a1

tune

0b13100

read concurrently

a54b3f0

tune

2ba2cc3

fix

7753867

small opt

4b876c9

D3Hunter added 2 commits May 15, 2025 19:40

lint

1464c50

read in serial, refactor

2aa096e

D3Hunter changed the title ~~[WIP]Parallel resolve conflicts | tidb-test=release-8.1.2~~ importinto: optimize conflicts resolution | tidb-test=release-8.1.2 May 15, 2025

ti-chi-bot bot removed do-not-merge/invalid-title do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels May 15, 2025

remove

a9b0589

joechenrh approved these changes May 16, 2025

View reviewed changes

Benjamin2037 reviewed May 16, 2025

View reviewed changes

Benjamin2037 approved these changes May 16, 2025

View reviewed changes

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

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 16, 2025

fix comments

c98e646

GMHDBJD approved these changes May 16, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label May 16, 2025

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label May 16, 2025

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 16, 2025

ti-chi-bot bot merged commit 8010c96 into pingcap:feature/release-8.1-gsort-test May 16, 2025
18 checks passed

D3Hunter deleted the parallel-resolve-conflicts branch May 16, 2025 11:06

D3Hunter added a commit to D3Hunter/tidb that referenced this pull request Nov 27, 2025

importinto: optimize conflicts resolution (pingcap#61059)

f8b838a

D3Hunter mentioned this pull request Nov 27, 2025

globalsort/importinto: pick conflict resolution, write subtask meta to global-sort store and other enhancements and fixes back to 8.5 #64710

Merged

13 tasks

importinto: optimize conflicts resolution | tidb-test=release-8.1.2 #61059

importinto: optimize conflicts resolution | tidb-test=release-8.1.2 #61059

Uh oh!

Conversation

D3Hunter commented May 9, 2025 • 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 May 9, 2025

Uh oh!

codecov bot commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

D3Hunter commented May 16, 2025

Uh oh!

tiprow bot commented May 16, 2025

Uh oh!

joechenrh left a comment

Choose a reason for hiding this comment

Uh oh!

joechenrh May 16, 2025

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented May 16, 2025

Uh oh!

Benjamin2037 left a comment

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented May 16, 2025

Uh oh!

ti-chi-bot bot commented May 16, 2025

Uh oh!

ti-chi-bot bot commented May 16, 2025

[LGTM Timeline notifier]

Uh oh!

D3Hunter commented May 16, 2025

Uh oh!

D3Hunter commented May 16, 2025

Uh oh!

tiprow bot commented May 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

D3Hunter commented May 9, 2025 •

edited

Loading

codecov bot commented May 9, 2025 •

edited

Loading