schema cache: record the ddl timestamp and associate schema cache with the timestamp for stale read by hihihuhu · Pull Request #42083 · pingcap/tidb

hihihuhu · 2023-03-09T22:17:16Z

What problem does this PR solve?

Issue Number: close #40740

Problem Summary:

What is changed and how it works?

when handle ddl, record the start ts of the transaction as the timestamp of the schema change. then when populate the schema cache, associate each schema version with a timestamp, and search the schema by timestamp for stale read.

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

ti-chi-bot · 2023-03-09T22:17:18Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

tiancaiamao
xhebox

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.

Details

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

ti-chi-bot · 2023-03-09T22:17:26Z

Hi @hihihuhu. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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

hihihuhu · 2023-03-13T16:47:50Z

@xhebox could you help to take a look when you have a chance? thanks!

xhebox

Maybe we should rename loadschema(startTS uint64) to loadschema(timestamp uint64). This function will accept a timestamp that starts with the desired schema and schemadiff/metadata. It does not mean that startTS is the timestamp set by updateSchemaVersion.

ddl/ddl.go

domain/domain.go

infoschema/cache.go

domain/domain.go

meta/meta.go

domain/domain.go

hihihuhu · 2023-03-21T19:50:01Z

thanks! i updated to store startTs in schemadiff and use it as the timestamp of a schema version, i haven't tested it yet but want to get your opinion first to see if this is what you expect?

meta/meta.go

hawkingrei · 2023-03-30T06:00:27Z

/ok-to-test

hawkingrei · 2023-03-30T06:00:38Z

/test all

hawkingrei · 2023-03-30T06:00:45Z

/merge

domain/domain.go

hawkingrei · 2023-03-30T07:12:39Z

@hihihuhu Please fix it.

owner/manager_test.go:58:63: too many arguments in call to ic.Insert

	have (infoschema.InfoSchema, number, number)

	want (infoschema.InfoSchema, uint64)

owner/manager_test.go:118:63: too many arguments in call to ic.Insert

	have (infoschema.InfoSchema, number, number)

	want (infoschema.InfoSchema, uint64)

owner/manager_test.go:134:64: too many arguments in call to ic2.Insert

	have (infoschema.InfoSchema, number, number)

	want (infoschema.InfoSchema, uint64)

owner/manager_test.go:159:64: too many arguments in call to ic3.Insert

	have (infoschema.InfoSchema, number, number)

	want (infoschema.InfoSchema, uint64)

hihihuhu · 2023-03-30T22:32:24Z

/retest

xhebox · 2023-03-31T03:32:37Z

/merge

ti-chi-bot · 2023-03-31T03:32:41Z

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

Details

Commit hash: dab3f0b

hihihuhu · 2023-07-25T18:01:44Z

@xhebox consider this is a fix of performance issue, is it possible to merge this into release-6.5? thanks!

xhebox · 2023-07-25T18:19:54Z

@xhebox consider this is a fix of performance issue, is it possible to merge this into release-6.5? thanks!

I think 'no'. We only pick fixes of bugs.

* cache schema version by timestamp * fix test * avoid recursive locking * address comment * optimize the code * fix 40740 * use the commit ts of the shcema diff as the timestamp of the schema * address comment * address comment * address comment * fix CI Signed-off-by: xhe <xw897002528@gmail.com> * fix CI Signed-off-by: xhe <xw897002528@gmail.com> * Update meta/meta.go * Update domain/domain.go * fix test --------- Signed-off-by: xhe <xw897002528@gmail.com> Co-authored-by: xhe <xw897002528@gmail.com> Co-authored-by: CbcWestwolf <1004626265@qq.com>

This reverts commit a85b8ed.

* cache schema version by timestamp * fix test * avoid recursive locking * address comment * optimize the code * fix 40740 * use the commit ts of the shcema diff as the timestamp of the schema * address comment * address comment * address comment * fix CI Signed-off-by: xhe <xw897002528@gmail.com> * fix CI Signed-off-by: xhe <xw897002528@gmail.com> * Update meta/meta.go * Update domain/domain.go * fix test --------- Signed-off-by: xhe <xw897002528@gmail.com> Co-authored-by: xhe <xw897002528@gmail.com> Co-authored-by: CbcWestwolf <1004626265@qq.com> fix wrong timestamp in schema cache address comment format

…h the timestamp for stale read (pingcap#42083) close pingcap#40740

…h the timestamp for stale read (#42083) (#46588) ref #43481

* cache schema version by timestamp * fix test * avoid recursive locking * address comment * optimize the code * fix 40740 * use the commit ts of the shcema diff as the timestamp of the schema * address comment * address comment * address comment * fix CI Signed-off-by: xhe <xw897002528@gmail.com> * fix CI Signed-off-by: xhe <xw897002528@gmail.com> * Update meta/meta.go * Update domain/domain.go * fix test --------- Signed-off-by: xhe <xw897002528@gmail.com> Co-authored-by: xhe <xw897002528@gmail.com> Co-authored-by: CbcWestwolf <1004626265@qq.com> fix wrong timestamp in schema cache address comment format Co-authored-by: Chen Ding <dingc05@gmail.com>

hihihuhu and others added 12 commits January 25, 2023 09:45

cache schema version by timestamp

5982d7c

fix test

b01a24b

avoid recursive locking

611f7e7

address comment

4da7ca7

optimize the code

1710996

Merge branch 'master' into master

3f9ce17

Merge branch 'master' into master

7145ef8

Merge branch 'master' into master

f804d62

Merge branch 'pingcap:master' into master

e37f6e8

Merge branch 'pingcap:master' into master

d7f7e18

fix 40740

cb4ed3e

Merge remote-tracking branch 'upstream/master'

541ccd5

xhebox reviewed Mar 16, 2023

View reviewed changes

ddl/ddl.go Outdated Show resolved Hide resolved

domain/domain.go Outdated Show resolved Hide resolved

infoschema/cache.go Outdated Show resolved Hide resolved

use the commit ts of the shcema diff as the timestamp of the schema

c169d46

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

hihihuhu commented Mar 17, 2023

View reviewed changes

domain/domain.go Outdated Show resolved Hide resolved

xhebox reviewed Mar 18, 2023

View reviewed changes

meta/meta.go Outdated Show resolved Hide resolved

domain/domain.go Outdated Show resolved Hide resolved

hihihuhu force-pushed the master branch 2 times, most recently from b16e8f5 to 8219173 Compare March 21, 2023 19:47

hihihuhu changed the title ~~[DDL] record the ddl timestamp and associate schema cache with the timestamp for stale read~~ schema cache: record the ddl timestamp and associate schema cache with the timestamp for stale read Mar 21, 2023

ti-chi-bot removed the do-not-merge/invalid-title label Mar 21, 2023

CbcWestwolf reviewed Mar 30, 2023

View reviewed changes

meta/meta.go Outdated Show resolved Hide resolved

Update meta/meta.go

6d78643

CbcWestwolf reviewed Mar 30, 2023

View reviewed changes

domain/domain.go Outdated Show resolved Hide resolved

Update domain/domain.go

9cf179e

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

hihihuhu force-pushed the master branch from 44b183c to e2b81a4 Compare March 30, 2023 20:15

fix test

dab3f0b

hihihuhu force-pushed the master branch from e2b81a4 to dab3f0b Compare March 30, 2023 21:16

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

ti-chi-bot merged commit 73d446e into pingcap:master Mar 31, 2023

hihihuhu added a commit to hihihuhu/tidb that referenced this pull request Jul 27, 2023

Revert "cherry-pick pingcap#42083 (#1)"

d6522c3

This reverts commit a85b8ed.

hihihuhu added a commit to hihihuhu/tidb that referenced this pull request Jul 27, 2023

Revert "cherry-pick pingcap#42083 (#1)" (#2)

317f529

This reverts commit a85b8ed.

crazycs520 pushed a commit to crazycs520/tidb that referenced this pull request Sep 1, 2023

schema cache: record the ddl timestamp and associate schema cache wit…

bb66596

…h the timestamp for stale read (pingcap#42083) close pingcap#40740

crazycs520 mentioned this pull request Sep 1, 2023

schema cache: record the ddl timestamp and associate schema cache with the timestamp for stale read (#42083) #46588

Merged

12 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Sep 1, 2023

schema cache: record the ddl timestamp and associate schema cache wit…

df1106e

…h the timestamp for stale read (#42083) (#46588) ref #43481

Conversation

hihihuhu commented Mar 9, 2023

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-chi-bot commented Mar 9, 2023

Uh oh!

hihihuhu commented Mar 13, 2023

Uh oh!

xhebox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hihihuhu commented Mar 21, 2023

Uh oh!

Uh oh!

hawkingrei commented Mar 30, 2023

Uh oh!

hawkingrei commented Mar 30, 2023

Uh oh!

hawkingrei commented Mar 30, 2023

Uh oh!

Uh oh!

hawkingrei commented Mar 30, 2023

Uh oh!

hihihuhu commented Mar 30, 2023

Uh oh!

xhebox commented Mar 31, 2023

Uh oh!

ti-chi-bot commented Mar 31, 2023

Uh oh!

hihihuhu commented Jul 25, 2023

Uh oh!

xhebox commented Jul 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ti-chi-bot commented Mar 9, 2023 •

edited

Loading