*: LOAD DATA support WITH DETACHED #42159

lance6716 · 2023-03-13T09:16:52Z

What problem does this PR solve?

Issue Number: ref #40499

Problem Summary:

What is changed and how it works?

Check List

Tests

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

MySQL [test]> load data infile 's3://load-csv/test.customer.aaaa.csv?access_key=xxx&secret_access_key=xxx' ignore into table my_table FIELDS TERMINATED BY ',' OPTIONALLY ENCLOSED BY '' LINES TERMINATED BY '\\n' IGNORE 1 LINES with detached;
+--------+
| Job_ID |
+--------+
|  90001 |
+--------+
1 row in set (2.651 sec)

MySQL [test]> select * from mysql.load_data_jobs where job_id = 90001\G
*************************** 1. row ***************************
         job_id: 90001
expected_status: running
    create_time: 2023-03-13 17:15:17.887803
     start_time: 2023-03-13 17:15:17.890134
    update_time: 2023-03-13 17:15:25.945554
       end_time: 2023-03-13 17:15:25.958597
    data_source: s3://load-csv/test.customer.aaaa.csv?access_key=xxx&secret_access_key=xxx
   table_schema: test
     table_name: my_table
    import_mode: logical
    create_user: root@%
       progress: {"SourceFileSize":52428118,"LoadedFileSize":52428118,"LoadedRowCnt":89281}
 result_message: Records: 89281  Deleted: 0  Skipped: 0  Warnings: 65535
  error_message: NULL

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

ti-chi-bot · 2023-03-13T09:16:54Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
hawkingrei

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.

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

lance6716 · 2023-03-14T03:08:27Z

/cc @D3Hunter @xhebox

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

lance6716 · 2023-03-14T04:59:07Z

/test build

D3Hunter · 2023-03-14T05:47:55Z

executor/builder.go

+	if v.Detached {
+		if v.FileLocRef == ast.FileLocClient {
+			b.err = exeerrors.ErrLoadDataCantDetachWithLocal
+			return nil


we can move loadDataController out so we use controller.Detached.

we can check this inside controller to make all checks in one place

D3Hunter · 2023-03-14T06:03:01Z

executor/load_data.go

@@ -245,6 +270,7 @@ type LoadDataWorker struct {
 	progress        atomic.Pointer[asyncloaddata.Progress]
 	getSysSessionFn func() (sessionctx.Context, error)
 	putSysSessionFn func(context.Context, sessionctx.Context)
+	ownSession      bool


reuse detached? always has same value with it

D3Hunter · 2023-03-14T06:06:18Z

executor/load_data.go

+) (err error) {
+	defer func() {
+		if e.ownSession {
+			e.putSysSessionFn(ctx, e.Ctx)


maybe remove e.Ctx, we're using e.ctx and e.Ctx mixingly, it's same as ctx in InsertValues

it's made public for LOAD DATA LOCAL where the clientConn.handleLoadData check Ctx.GetSessionVars().Killed to handle reading packet. Maybe I can unify the CANCEL LOAD DATA JOB with this checking killed in next PR.

D3Hunter · 2023-03-14T06:53:38Z

executor/load_data.go

+		}
+	}()
+
+	s, err := e.getSysSessionFn()


why get a new session to execute job related sql?

D3Hunter · 2023-03-14T06:55:24Z

executor/load_data.go


 	defer func() {
+		ctx2 := context.Background()


make it work even ctx is cancelled? maybe leave a comment

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

D3Hunter · 2023-03-14T08:35:03Z

planner/core/planbuilder.go

@@ -4238,6 +4248,12 @@ func (b *PlanBuilder) buildLoadData(ctx context.Context, ld *ast.LoadDataStmt) (
 	if err != nil {
 		return nil, err
 	}
+
+	if detached {


can we move this into buildLoadData, so no need to init `detached

This is already buildLoadData. You mean buildLoadData (executor)?

~~executor builder can not change the schema of plan, it's one way~~

I moved it into buildLoadData (executor) and the query schema is not changed. Maybe other components have already read the old value and decided the query output before build executor.

D3Hunter · 2023-03-14T08:35:56Z

/hold

lance6716 · 2023-03-14T09:10:24Z

/unhold

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

ti-chi-bot · 2023-03-15T05:03:02Z

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

Commit hash: f92a8b3

lance6716 · 2023-03-15T05:29:51Z

/test unit-test

lance6716 · 2023-03-15T06:16:50Z

/retest

lance6716 · 2023-03-15T06:51:49Z

/test unit-test

ti-chi-bot · 2023-03-15T06:51:51Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test unit-test

Use /test all to run all jobs.

In response to this:

/test unit-test

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/test-infra repository.

lance6716 · 2023-03-15T06:52:19Z

/test unit-test

ti-chi-bot · 2023-03-15T06:52:21Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test unit-test

Use /test all to run all jobs.

In response to this:

/test unit-test

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/test-infra repository.

lance6716 · 2023-03-15T06:52:30Z

/test unit-test

ti-chi-bot · 2023-03-15T06:52:32Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test unit-test

Use /test all to run all jobs.

In response to this:

/test unit-test

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/test-infra repository.

lance6716 · 2023-03-15T06:55:38Z

/test unit-test

ti-chi-bot · 2023-03-15T06:55:40Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test unit-test

Use /test all to run all jobs.

In response to this:

/test unit-test

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/test-infra repository.

lance6716 · 2023-03-15T06:58:45Z

/test unit-test

lance6716 · 2023-03-15T07:54:36Z

/retest

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

lance6716 · 2023-03-15T08:47:28Z

/merge

ti-chi-bot · 2023-03-15T08:47:32Z

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

Commit hash: 7752768

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

lance6716 · 2023-03-15T09:20:57Z

/unhold
/merge

ti-chi-bot · 2023-03-15T09:21:01Z

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

Commit hash: 1831183

lance6716 · 2023-03-15T11:24:04Z

/retest

--wip-- [skip ci]

4e6a559

ti-chi-bot added release-note-none do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2023

lance6716 added 3 commits March 13, 2023 22:46

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

9d20a30

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

sync code

69d6f09

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

fix

546b044

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

lance6716 changed the title ~~[WIP] *: LOAD DATA support WITH DETACHED~~ *: LOAD DATA support WITH DETACHED Mar 14, 2023

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 Mar 14, 2023

lance6716 added 2 commits March 14, 2023 10:55

fix UT

e6d8952

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

add test about IGNORE/REPLACE

51a899a

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

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 14, 2023

ti-chi-bot requested review from D3Hunter and xhebox March 14, 2023 03:08

make errdoc

b10d2ea

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

D3Hunter reviewed Mar 14, 2023

View reviewed changes

lance6716 added 2 commits March 14, 2023 15:37

address comment

4fa316e

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

remove unneeded field

00ec1e7

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

D3Hunter approved these changes Mar 14, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 14, 2023

D3Hunter reviewed Mar 14, 2023

View reviewed changes

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

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

remove unused field

a1db2e3

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

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 15, 2023

lance6716 added 2 commits March 15, 2023 16:46

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

a83db5b

skip a UT

7752768

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

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 15, 2023

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 15, 2023

hawkingrei added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2023

try to fix unstable UT

1831183

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

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 15, 2023

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

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 15, 2023

ti-chi-bot merged commit 982e0dc into pingcap:master Mar 15, 2023

lance6716 mentioned this pull request Mar 16, 2023

Unstable test TestCartesianJoinPanic #42250

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: LOAD DATA support WITH DETACHED #42159

*: LOAD DATA support WITH DETACHED #42159

lance6716 commented Mar 13, 2023 •

edited

ti-chi-bot commented Mar 13, 2023 •

edited

lance6716 commented Mar 14, 2023

lance6716 commented Mar 14, 2023

D3Hunter Mar 14, 2023

D3Hunter Mar 14, 2023

D3Hunter Mar 14, 2023

lance6716 Mar 14, 2023

D3Hunter Mar 14, 2023

D3Hunter Mar 14, 2023

lance6716 Mar 14, 2023

D3Hunter Mar 14, 2023

lance6716 Mar 14, 2023 •

edited

lance6716 Mar 14, 2023 •

edited

D3Hunter commented Mar 14, 2023

lance6716 commented Mar 14, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

*: LOAD DATA support WITH DETACHED #42159

*: LOAD DATA support WITH DETACHED #42159

Conversation

lance6716 commented Mar 13, 2023 • edited

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Mar 13, 2023 • edited

lance6716 commented Mar 14, 2023

lance6716 commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lance6716 Mar 14, 2023 • edited

Choose a reason for hiding this comment

lance6716 Mar 14, 2023 • edited

Choose a reason for hiding this comment

D3Hunter commented Mar 14, 2023

lance6716 commented Mar 14, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

ti-chi-bot commented Mar 15, 2023

lance6716 commented Mar 15, 2023

lance6716 commented Mar 13, 2023 •

edited

ti-chi-bot commented Mar 13, 2023 •

edited

lance6716 Mar 14, 2023 •

edited

lance6716 Mar 14, 2023 •

edited