*: caller can use a fixed TS for fast reorg #52993

lance6716 · 2024-04-29T14:38:36Z

What problem does this PR solve?

Issue Number: ref #49233 close #46986

Problem Summary:

What changed and how does it work?

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.

GOCACHE=/tmp/failpoint-cache go build -o bin/tidb-server-failpoint -toolexec failpoint-toolexec ./cmd/tidb-server

tiup playground nightly --tiflash 0 --without-monitor --db 0

rm /tmp/tidb-{0,1}.log

GO_FAILPOINTS="github.com/pingcap/tidb/pkg/ddl/ingest/resignAfterFlush=return();github.com/pingcap/tidb/pkg/ddl/ingest/checkpointLoopExit=return();github.com/pingcap/tidb/pkg/ddl/ingest/forceSyncFlagForTest=return();github.com/pingcap/tidb/pkg/ddl/ownerResignAfterDispatchLoopCheck=return()" ./bin/tidb-server-failpoint -P 4000 --store=tikv --host=127.0.0.1 --status=10080 --path=127.0.0.1:2379 --log-file=/tmp/tidb-0.log

mysql --comments --host 127.0.0.1 --port 4000 -u root -e "set @@global.tidb_enable_dist_task = 0;set @@global.tidb_ddl_reorg_worker_cnt = 1;create table test.t(c int primary key, c2 int);split table test.t between (0) and (10000) regions 10;insert into test.t values (1,1), (3000, 3000), (6000, 6000);"

./bin/tidb-server-failpoint -P 4001 --store=tikv --host=127.0.0.1 --status=10081 --path=127.0.0.1:2379 --log-file=/tmp/tidb-1.log

mysql --comments --host 127.0.0.1 --port 4001 -u root -e "alter table test.t add unique index idx(c2);"

before

after

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 <[email protected]>

tiprow · 2024-04-29T14:39:56Z

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

codecov · 2024-04-29T14:54:41Z

Codecov Report

Attention: Patch coverage is 57.40741% with 69 lines in your changes are missing coverage. Please review.

Project coverage is 73.8742%. Comparing base (49f09fe) to head (866b8ab).

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #52993        +/-   ##
================================================
+ Coverage   72.4413%   73.8742%   +1.4328%     
================================================
  Files          1492       1533        +41     
  Lines        429013     444320     +15307     
================================================
+ Hits         310783     328238     +17455     
+ Misses        98955      95419      -3536     
- Partials      19275      20663      +1388

Flag	Coverage Δ
integration	`49.3033% <52.4691%> (?)`
unit	`71.2363% <34.8101%> (-0.0962%)`	⬇️

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

Components	Coverage Δ
dumpling	`53.9957% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`46.6467% <ø> (+5.2232%)`	⬆️

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

lance6716 · 2024-05-08T04:02:38Z

/retest

tiprow · 2024-05-08T04:03:12Z

@lance6716: 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-sigs/prow repository.

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

pkg/ddl/backfilling_dist_scheduler.go

pkg/lightning/backend/local/engine_mgr.go

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

D3Hunter

rest lgtm

D3Hunter · 2024-05-10T09:21:36Z

pkg/lightning/backend/local/engine_mgr.go

-		ts := oracle.ComposeTS(physical, logical)
+		ts := cfg.TS
+		if ts == 0 {
+			ts = oracle.ComposeTS(physical, logical)


we can skip GetTS if ts != 0

D3Hunter · 2024-05-10T09:24:47Z

pkg/lightning/backend/backend.go

@@ -89,6 +89,9 @@ type EngineConfig struct {
 	// KeepSortDir indicates whether to keep the temporary sort directory
 	// when opening the engine, instead of removing it.
 	KeepSortDir bool
+	// TS is the preset timestamp of data in the engine. When it's 0, the used TS
+	// will be set lazily.


Suggested change

// will be set lazily.

// will be set lazily.

// only used in global sort now

it's also used in generateLocalEngineConfig in local fast reorg of DDL

D3Hunter · 2024-05-10T09:39:20Z

pkg/ddl/ingest/checkpoint.go

@@ -298,6 +314,7 @@ type ReorgCheckpoint struct {
 	PhysicalID int64  `json:"physical_id"`
 	StartKey   kv.Key `json:"start_key"`
 	EndKey     kv.Key `json:"end_key"`
+	TS         uint64 `json:"ts"`


Suggested change

TS uint64 `json:"ts"`

// TS of next engine ingest.

TS uint64 `json:"ts"`

D3Hunter · 2024-05-10T09:44:19Z

pkg/ddl/ingest/checkpoint.go

+	return nil
+}
+
+func (s *CheckpointManager) refreshTS() (uint64, error) {


Suggested change

func (s *CheckpointManager) refreshTS() (uint64, error) {

func (s *CheckpointManager) refreshTSAndUpdateCP() (uint64, error) {

the refreshTS part is same as in resumeOrInitCheckpoint

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

ti-chi-bot · 2024-05-10T09:57:18Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,tangenta]
~~pkg/ddl/OWNERS~~ [tangenta]
~~pkg/disttask/OWNERS~~ [D3Hunter,tangenta]
~~pkg/lightning/OWNERS~~ [D3Hunter]

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 · 2024-05-10T09:57:21Z

[LGTM Timeline notifier]

Timeline:

2024-05-10 08:13:37.757198525 +0000 UTC m=+1208971.514334083: ☑️ agreed by tangenta.
2024-05-10 09:57:20.835816503 +0000 UTC m=+1215194.592952077: ☑️ agreed by D3Hunter.

lance6716 · 2024-05-10T12:42:17Z

/hold

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

lance6716 · 2024-05-10T12:47:00Z

/unhold

lance6716 · 2024-05-11T02:21:38Z

/cherry-pick release-8.1

close pingcap#46986, ref pingcap#49233

ti-chi-bot · 2024-05-11T02:22:20Z

@lance6716: new pull request created to branch release-8.1: #53188.

In response to this:

/cherry-pick release-8.1

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.

close #46986, ref #49233

close pingcap#46986, ref pingcap#49233

*: add TS field so caller can use a fixed TS

db315e9

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

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 29, 2024

lance6716 added 2 commits April 30, 2024 14:02

update

819b480

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

add reproduce

6e71d14

Signed-off-by: lance6716 <[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 Apr 30, 2024

lance6716 mentioned this pull request Apr 30, 2024

duplicate error message for ADD UNIQUE INDEX has wrong format #53004

Closed

lance6716 added 5 commits May 7, 2024 18:53

Merge branch 'master' of github.com:pingcap/tidb into open-engine-ts

476b9e8

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

add global sort impl

d2ed1c6

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

fix CI

68e2556

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

make bazel

9893308

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

implement for local fast reorg

1b5ea01

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

ti-chi-bot 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 May 8, 2024

lance6716 changed the title ~~[WIP]*: add TS field so caller can use a fixed TS~~ *: add TS field so caller can use a fixed TS May 8, 2024

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

fix UT

9bbd8c7

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

lance6716 changed the title *: add TS field so caller can use a fixed TS *: caller can use a fixed TS for fast reorg May 8, 2024

lance6716 changed the title *: caller can use a fixed TS for fast reorg [WIP]*: caller can use a fixed TS for fast reorg May 8, 2024

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2024

lance6716 added 2 commits May 8, 2024 11:21

fix bazel

42c0755

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

try to fix CI

b73d73e

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

lance6716 changed the title ~~[WIP]*: caller can use a fixed TS for fast reorg~~ *: caller can use a fixed TS for fast reorg May 8, 2024

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

rename

1de9585

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

tangenta reviewed May 10, 2024

View reviewed changes

pkg/ddl/backfilling_dist_scheduler.go Show resolved Hide resolved

tangenta reviewed May 10, 2024

View reviewed changes

pkg/lightning/backend/local/engine_mgr.go Show resolved Hide resolved

address comment

8f94822

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

tangenta approved these changes May 10, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label May 10, 2024

D3Hunter reviewed May 10, 2024

View reviewed changes

address comment

096a981

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

D3Hunter approved these changes May 10, 2024

View reviewed changes

ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 10, 2024

Merge branch 'master' of github.com:pingcap/tidb into open-engine-ts

7abcef1

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 10, 2024

fix CI

866b8ab

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

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 10, 2024

lance6716 mentioned this pull request May 10, 2024

ddl: fast reorg of index may cause duplicate entry error #49233

Open

ti-chi-bot bot merged commit f46db20 into pingcap:master May 10, 2024
22 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request May 11, 2024

*: caller can use a fixed TS for fast reorg (pingcap#52993)

d82de1a

close pingcap#46986, ref pingcap#49233

ti-chi-bot mentioned this pull request May 11, 2024

*: caller can use a fixed TS for fast reorg (#52993) #53188

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request May 11, 2024

*: caller can use a fixed TS for fast reorg (#52993) (#53188)

232c0e0

close #46986, ref #49233

terry1purcell pushed a commit to terry1purcell/tidb that referenced this pull request May 17, 2024

*: caller can use a fixed TS for fast reorg (pingcap#52993)

0fc6984

close pingcap#46986, ref pingcap#49233

RidRisR pushed a commit to RidRisR/tidb that referenced this pull request May 23, 2024

*: caller can use a fixed TS for fast reorg (pingcap#52993)

3f928cb

close pingcap#46986, ref pingcap#49233

lance6716 mentioned this pull request Dec 4, 2024

non-DXF ADD INDEX of large table (trigger import due to disk quota) will write KV with TS = 0 #57980

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: caller can use a fixed TS for fast reorg #52993

*: caller can use a fixed TS for fast reorg #52993

lance6716 commented Apr 29, 2024 •

edited

Loading

tiprow bot commented Apr 29, 2024

codecov bot commented Apr 29, 2024 •

edited

Loading

lance6716 commented May 8, 2024

tiprow bot commented May 8, 2024

D3Hunter left a comment

D3Hunter May 10, 2024

D3Hunter May 10, 2024

lance6716 May 10, 2024

D3Hunter May 10, 2024

D3Hunter May 10, 2024

ti-chi-bot bot commented May 10, 2024

ti-chi-bot bot commented May 10, 2024

lance6716 commented May 10, 2024

lance6716 commented May 10, 2024

lance6716 commented May 11, 2024

ti-chi-bot commented May 11, 2024

	// will be set lazily.
	// will be set lazily.
	// only used in global sort now

	TS uint64 `json:"ts"`
	// TS of next engine ingest.
	TS uint64 `json:"ts"`

	func (s *CheckpointManager) refreshTS() (uint64, error) {
	func (s *CheckpointManager) refreshTSAndUpdateCP() (uint64, error) {

*: caller can use a fixed TS for fast reorg #52993

*: caller can use a fixed TS for fast reorg #52993

Conversation

lance6716 commented Apr 29, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Apr 29, 2024

codecov bot commented Apr 29, 2024 • edited Loading

Codecov Report

lance6716 commented May 8, 2024

tiprow bot commented May 8, 2024

D3Hunter left a comment

Choose a reason for hiding this comment

D3Hunter May 10, 2024

Choose a reason for hiding this comment

D3Hunter May 10, 2024

Choose a reason for hiding this comment

lance6716 May 10, 2024

Choose a reason for hiding this comment

D3Hunter May 10, 2024

Choose a reason for hiding this comment

D3Hunter May 10, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented May 10, 2024

ti-chi-bot bot commented May 10, 2024

[LGTM Timeline notifier]

lance6716 commented May 10, 2024

lance6716 commented May 10, 2024

lance6716 commented May 11, 2024

ti-chi-bot commented May 11, 2024

lance6716 commented Apr 29, 2024 •

edited

Loading

codecov bot commented Apr 29, 2024 •

edited

Loading