ddl, lightning: integrate operator for global sort #46734

tangenta · 2023-09-06T19:08:25Z

What problem does this PR solve?

Issue Number: ref #45719

Problem Summary:

What is changed and how it works?

Check List

Tests

Unit test
Integration test

Manual test (add detailed scripts or steps below)

Create a tiup cluster:

tiup playground nightly --db 0 --kv 1 --pd 1  --without-monitor

Add three tidb servers:

./bin/tidb-server -P 4000 --store=tikv --host=127.0.0.1 --status=10080 --path=127.0.0.1:2379 --log-file=tidb-0.log
./bin/tidb-server -P 4001 --store=tikv --host=127.0.0.1 --status=10081 --path=127.0.0.1:2379 --log-file=tidb-1.log
./bin/tidb-server -P 4002 --store=tikv --host=127.0.0.1 --status=10082 --path=127.0.0.1:2379 --log-file=tidb-2.log

Insert 10 million rows:

sysbench --test=oltp_read_write --table-size=10000000 --db-driver=mysql --mysql-user=root --mysql-password='' --mysql-host=127.0.0.1 --mysql-port=4000 --mysql-db=test --threads=4 --time=1800 --report-interval=1 prepare
WARNING: the --test option is deprecated. You can pass a script name or path on the command line without any options.
sysbench 1.0.20 (using system LuaJIT 2.1.0-beta3)

Initializing worker threads...

Creating table 'sbtest1'...
Inserting 10000000 records into 'sbtest1'
Creating a secondary index on 'sbtest1'...

Start minio server and create a bucket 'globalsort':

./minio server ./data

Create a bucket through minio web interface.
Enable dist task and set system variable tidb_cloud_storage_uri:

mysql> set @@global.tidb_enable_dist_task = 1;
Query OK, 0 rows affected (0.01 sec)
mysql> set @@global.tidb_cloud_storage_uri = "s3://globalsort/addindex?access-key=minioadmin&secret-access-key=minioadmin&endpoint=http://127.0.0.1:9000&force-path-style=true";
Query OK, 0 rows affected (0.01 sec)

Add index for sbtest1:

mysql> alter table sbtest1 add index idx(k);
Query OK, 0 rows affected (5.87 sec)

Show jobs and admin check:

mysql> admin show ddl jobs;
+--------+---------+-------------------------+------------------------+--------------+-----------+----------+-----------+---------------------+---------------------+---------------------+--------+
| JOB_ID | DB_NAME | TABLE_NAME              | JOB_TYPE               | SCHEMA_STATE | SCHEMA_ID | TABLE_ID | ROW_COUNT | CREATE_TIME         | START_TIME          | END_TIME            | STATE  |
+--------+---------+-------------------------+------------------------+--------------+-----------+----------+-----------+---------------------+---------------------+---------------------+--------+
|    103 | test    | sbtest1                 | add index /* ingest */ | public       |         2 |      100 |  10000000 | 2023-09-12 16:27:35 | 2023-09-12 16:27:35 | 2023-09-12 16:27:41 | synced |
...
+--------+---------+-------------------------+------------------------+--------------+-----------+----------+-----------+---------------------+---------------------+---------------------+--------+
10 rows in set (0.02 sec)

mysql> admin check table sbtest1;
Query OK, 0 rows affected (4.20 sec)

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>

tiprow · 2023-09-06T19:08:47Z

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

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

tangenta · 2023-09-07T08:25:34Z

Wait for #46636 and #46407

D3Hunter

29/32

disttask/framework/scheduler/execute/interface.go

D3Hunter · 2023-09-11T14:14:20Z

ddl/stage_merge_sort.go

+	if err != nil {
+		return nil, err
+	}
+	err = local.ImportEngine(ctx, engineUUID, int64(config.SplitRegionSize), int64(config.SplitRegionKeys))


from what this stage does, should it be named as write&ingest stage?

@ywqzzy changed the behavior of framework in #46593. Maybe we need to discuss how to operate the stages.

D3Hunter

so in this pr, there is no merge sort range overlapped files part?

tangenta · 2023-09-12T04:32:44Z

@D3Hunter Mulitple-pass merge sort will be implemented in next PR.

D3Hunter · 2023-09-12T05:17:22Z

ddl/backfilling_dispatcher.go

-			// This flow for partition table has only one step
+		switch gTask.Step {
+		case stageInit:
+			gTask.Step = stageReadIndex


framework also changes step, we need unify where we can change steps, see https://github.com/pingcap/tidb/pull/46593/files#diff-2a828989b540648a0bdef3d984d7946ea4fcb3c18718a4047b320c712ee1b855R424

framework also changes step, we need unify where we can change steps, see https://github.com/pingcap/tidb/pull/46593/files#diff-2a828989b540648a0bdef3d984d7946ea4fcb3c18718a4047b320c712ee1b855R424

I'm trying to change it.

D3Hunter · 2023-09-12T07:34:20Z

ddl/backfilling_dispatcher.go

+			if err := taskHandle.UpdateTask(gTask.State, nil, dispatcher.RetrySQLTimes); err != nil {
+				logutil.Logger(ctx).Error("failed to UpdateTask", zap.Error(err))


seems this changes nothing? @ywqzzy

I will move this to framework in the next pr

as discussed, we will move all task status change to framework later. cc @ywqzzy

lance6716 · 2023-09-12T06:28:39Z

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

+// LockForWrite implements ingest.Writer.
+// Since flushKVs is thread-safe in external storage writer,
+// this is implemented as noop.
+func (w *Writer) LockForWrite() func() {


I think we can provide a batch version WriteRows. We can discuss it later.

ti-chi-bot · 2023-09-12T07:36:55Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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~~ [D3Hunter,lance6716]

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 · 2023-09-12T07:36:59Z

[LGTM Timeline notifier]

Timeline:

2023-09-12 07:28:44.791628116 +0000 UTC m=+340706.716184507: ☑️ agreed by D3Hunter.
2023-09-12 07:36:57.894256373 +0000 UTC m=+341199.818812768: ☑️ agreed by lance6716.

D3Hunter · 2023-09-12T07:37:12Z

/hold

D3Hunter · 2023-09-12T07:41:22Z

/unhold

ywqzzy · 2023-09-12T08:38:59Z

/retest

tiprow · 2023-09-12T08:39:32Z

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

lance6716 added 12 commits September 4, 2023 19:15

lightning: add ExternalEngineConfig to create an external engine

b82492e

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

add two more config

ba62aee

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

fix UT

fc7e903

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

fix bazel

3a5da7a

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

fix lint

1ede83e

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

add UT

0c60d4c

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

fix sec lint

faae105

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

fix bazel

549d0da

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

address comment

2c6257b

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

have to use failpoint

ea7494d

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

fix CI

7f7bef5

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

address comment

4a89257

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

lance6716 and others added 3 commits September 7, 2023 11:18

switch to failpoint

f6de222

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

Merge branch 'master' of github.com:pingcap/tidb into external-config

820326d

ddl, lightning: integrate operator for global sort

83ec65d

tangenta force-pushed the global-sort-operator branch from 07bd61f to 83ec65d Compare September 7, 2023 08:02

ti-chi-bot bot removed do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Sep 7, 2023

update bazel

6f098c4

tangenta added 4 commits September 7, 2023 17:23

use jobID as external storage prefix

be3e412

Merge remote-tracking branch 'upstream/master' into global-sort-operator

feb25b1

fix file prefix and linter

8791caa

fix linter

e093425

D3Hunter reviewed Sep 11, 2023

View reviewed changes

disttask/framework/scheduler/execute/interface.go Show resolved Hide resolved

tangenta added 3 commits September 11, 2023 20:12

address comments

ba083ae

Merge remote-tracking branch 'upstream/master' into HEAD

5aa5e09

apply tidb_cloud_storage_uri

5c209b5

D3Hunter reviewed Sep 11, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/master' into HEAD

56a7378

D3Hunter reviewed Sep 12, 2023

View reviewed changes

ywqzzy and others added 3 commits September 12, 2023 14:35

fix (#127)

271d1fd

Merge remote-tracking branch 'upstream/master' into HEAD

d9e6bce

collect min max key correctly

a19bd15

D3Hunter approved these changes Sep 12, 2023

View reviewed changes

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

add comment

5ab3433

D3Hunter reviewed Sep 12, 2023

View reviewed changes

lance6716 approved these changes Sep 12, 2023

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 12, 2023

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 Sep 12, 2023

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 Sep 12, 2023

ti-chi-bot bot merged commit be2a243 into pingcap:master Sep 12, 2023
16 of 20 checks passed

ywqzzy mentioned this pull request Sep 14, 2023

Enhance distributed task execution framework #46258

Closed

76 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl, lightning: integrate operator for global sort #46734

ddl, lightning: integrate operator for global sort #46734

tangenta commented Sep 6, 2023 •

edited

Loading

tiprow bot commented Sep 6, 2023

tangenta commented Sep 7, 2023

D3Hunter left a comment

D3Hunter Sep 11, 2023

tangenta Sep 12, 2023

D3Hunter left a comment

tangenta commented Sep 12, 2023

D3Hunter Sep 12, 2023

ywqzzy Sep 12, 2023

D3Hunter Sep 12, 2023

ywqzzy Sep 12, 2023

D3Hunter Sep 12, 2023

lance6716 Sep 12, 2023

ti-chi-bot bot commented Sep 12, 2023

ti-chi-bot bot commented Sep 12, 2023

D3Hunter commented Sep 12, 2023

D3Hunter commented Sep 12, 2023

ywqzzy commented Sep 12, 2023

tiprow bot commented Sep 12, 2023

		if err := taskHandle.UpdateTask(gTask.State, nil, dispatcher.RetrySQLTimes); err != nil {
		logutil.Logger(ctx).Error("failed to UpdateTask", zap.Error(err))

ddl, lightning: integrate operator for global sort #46734

ddl, lightning: integrate operator for global sort #46734

Conversation

tangenta commented Sep 6, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

tiprow bot commented Sep 6, 2023

tangenta commented Sep 7, 2023

D3Hunter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

D3Hunter left a comment

Choose a reason for hiding this comment

tangenta commented Sep 12, 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

ti-chi-bot bot commented Sep 12, 2023

ti-chi-bot bot commented Sep 12, 2023

[LGTM Timeline notifier]

D3Hunter commented Sep 12, 2023

D3Hunter commented Sep 12, 2023

ywqzzy commented Sep 12, 2023

tiprow bot commented Sep 12, 2023

tangenta commented Sep 6, 2023 •

edited

Loading