infoschema: fix issue of information schema cache miss cause by schema version gap #53445

crazycs520 · 2024-05-21T09:42:22Z

What problem does this PR solve?

Issue Number: close #53428

Problem Summary: fix issue of information schema cache miss cause by schema version gap

What changed and how does it work?

before this PR

When DDL has empty schema version, will cause cache miss, then stale-read query need to load snapshot schema from TiKV.

[2024/05/23 06:15:10.546 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6760] [neededSchemaVersion=6762] ["start time"=2.583049ms] [gotSchemaVersion=6762] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:15:17.869 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6763] [neededSchemaVersion=6765] ["start time"=2.040666ms] [gotSchemaVersion=6765] [phyTblIDs="[10496]"] [actionTypes="[4]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:15:18.051 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6765] [neededSchemaVersion=6767] ["start time"=4.321375ms] [gotSchemaVersion=6767] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:15:18.245 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6767] [neededSchemaVersion=6769] ["start time"=3.01603ms] [gotSchemaVersion=6769] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]

This PR

When DDL has empty schema version, this PR won't have schema cache miss, then won't cause many load snapshot schema operation.

[2024/05/23 06:23:45.209 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6777] [neededSchemaVersion=6779] ["start time"=5.409896ms] [gotSchemaVersion=6779] [phyTblIDs="[10502]"] [actionTypes="[4]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:45.458 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6779] [neededSchemaVersion=6781] ["start time"=2.711853ms] [gotSchemaVersion=6781] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:45.650 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6781] [neededSchemaVersion=6783] ["start time"=1.910751ms] [gotSchemaVersion=6783] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:49.173 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6784] [neededSchemaVersion=6786] ["start time"=2.511772ms] [gotSchemaVersion=6786] [phyTblIDs="[10505]"] [actionTypes="[4]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:49.372 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6786] [neededSchemaVersion=6788] ["start time"=2.768253ms] [gotSchemaVersion=6788] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:49.590 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6788] [neededSchemaVersion=6790] ["start time"=4.466332ms] [gotSchemaVersion=6790] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:53.670 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6791] [neededSchemaVersion=6793] ["start time"=799.389µs] [gotSchemaVersion=6793] [phyTblIDs="[10508]"] [actionTypes="[4]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:53.872 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6793] [neededSchemaVersion=6795] ["start time"=1.025364ms] [gotSchemaVersion=6795] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]
[2024/05/23 06:23:54.090 +00:00] [INFO] [domain.go:287] ["diff load InfoSchema success"] [currentSchemaVersion=6795] [neededSchemaVersion=6797] ["start time"=3.549838ms] [gotSchemaVersion=6797] [phyTblIDs="[]"] [actionTypes="[]"] [diffTypes="[\"drop table\"]"]

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.

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

…a version gap Signed-off-by: crazycs520 <crazycs520@gmail.com>

ti-chi-bot · 2024-05-21T09:42:25Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tiprow · 2024-05-21T09:42:38Z

Hi @crazycs520. 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-sigs/prow repository.

crazycs520 · 2024-05-21T09:44:13Z

@lcwangchao @zimulala @tangenta PTAL

codecov · 2024-05-21T10:01:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.0142%. Comparing base (53bf76f) to head (f878627).
Report is 24 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #53445        +/-   ##
================================================
+ Coverage   72.5441%   75.0142%   +2.4700%     
================================================
  Files          1505       1505                
  Lines        429846     432958      +3112     
================================================
+ Hits         311828     324780     +12952     
+ Misses        98749      88376     -10373     
- Partials      19269      19802       +533

Flag	Coverage Δ
integration	`49.8254% <58.6956%> (?)`
unit	`71.7725% <100.0000%> (+0.3376%)`	⬆️

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

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

wjhuang2016 · 2024-05-21T10:09:26Z

pkg/infoschema/cache.go

+			versions = append(versions, ver)
+		}
+		sort.Slice(versions, func(i, j int) bool { return versions[i] < versions[j] })
+		for _, ver := range versions {


Is removing one version enough?

Because empty schema version rarely appears, I think it's okay to delete just one here.

…-schema-cahce-hole

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

pkg/infoschema/cache.go

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

crazycs520 · 2024-05-21T17:39:13Z

/retest-required

tiprow · 2024-05-21T17:40:52Z

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

In response to this:

/retest-required

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 · 2024-05-22T03:23:05Z

pkg/infoschema/cache.go

+
+	h.emptySchemaVersions[version] = struct{}{}
+	if len(h.emptySchemaVersions) > cap(h.cache) {
+		// remove oldest version.


without statistics of how those version are accessed, remove oldest is no better than remove random version, and more complex

For stale-read query workload, it will always need recent schema version, so remove the oldest version is better.

tiancaiamao

How about insert a clone of previous infoschema for the empty schema diff? with its schema version = previous schema version + 1
The data structure and all the other operations can be the same as before.

ti-chi-bot · 2024-05-22T14:03:55Z

[LGTM Timeline notifier]

Timeline:

2024-05-21 12:12:49.549483541 +0000 UTC m=+2173723.306619106: ☑️ agreed by lcwangchao.
2024-05-22 14:03:54.117975959 +0000 UTC m=+2266787.875111530: ☑️ agreed by tiancaiamao.

crazycs520 · 2024-05-23T06:33:27Z

How about insert a clone of previous infoschema for the empty schema diff? with its schema version = previous schema version + 1 The data structure and all the other operations can be the same as before.

Nice catch, I try to use methold, but this will need to introduce another TiKV RPC to get the schemaTS of the empty schema version.

crazycs520 · 2024-05-23T07:58:54Z

/retest-required

…a version gap (pingcap#53445) close pingcap#53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-05-27T09:00:32Z

In response to a cherrypick label: new pull request created to branch release-6.5: #53583.

ti-chi-bot · 2024-05-27T09:00:34Z

In response to a cherrypick label: new pull request created to branch release-8.1: #53584.

ti-chi-bot · 2024-05-27T09:00:39Z

In response to a cherrypick label: new pull request created to branch release-7.1: #53585.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-05-27T09:00:41Z

In response to a cherrypick label: new pull request created to branch release-7.5: #53586.

This reverts commit 0771279.

…a version gap (pingcap#53445) close pingcap#53428

…a version gap (pingcap#53445) close pingcap#53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

This reverts commit 2653442.

…a version gap (pingcap#53445) close pingcap#53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

…a version gap (#53445) (#53586) close #53428

…a version gap (#53445) (#53562) * infoschema: fix issue of information schema cache miss cause by schema version gap (#53445) close #53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

…a version gap (#53445) (#53583) close #53428

…a version gap (#53445) (#53585) close #53428

…a version gap (pingcap#53445) (pingcap#53583) (pingcap#97) close pingcap#53428 Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

…a version gap (#53445) (#53584) close #53428

infoschema: fix issue of information schema cache miss cause by schem…

f528f02

…a version gap Signed-off-by: crazycs520 <crazycs520@gmail.com>

crazycs520 marked this pull request as ready for review May 21, 2024 09:43

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

wjhuang2016 reviewed May 21, 2024

View reviewed changes

Merge branch 'master' of https://github.com/pingcap/tidb into fix-buf…

56d4904

…-schema-cahce-hole

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label May 21, 2024

crazycs520 added 2 commits May 21, 2024 19:13

make bazel_prepare

54d4508

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

fix ci

6eda953

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

lcwangchao approved these changes May 21, 2024

View reviewed changes

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

wjhuang2016 reviewed May 21, 2024

View reviewed changes

pkg/infoschema/cache.go Show resolved Hide resolved

add log

f878627

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

D3Hunter reviewed May 22, 2024

View reviewed changes

tiancaiamao reviewed May 22, 2024

View reviewed changes

tiancaiamao approved these changes May 22, 2024

View reviewed changes

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

ti-chi-bot bot merged commit 0ac2ad0 into pingcap:master May 27, 2024
23 checks passed

crazycs520 added a commit to crazycs520/tidb that referenced this pull request May 27, 2024

infoschema: fix issue of information schema cache miss cause by schem…

339d391

…a version gap (pingcap#53445) close pingcap#53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

crazycs520 mentioned this pull request May 27, 2024

infoschema: fix issue of information schema cache miss cause by schema version gap (#53445) #53562

Merged

13 tasks

crazycs520 deleted the fix-buf-schema-cahce-hole branch May 27, 2024 08:59

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

This is an automated cherry-pick of pingcap#53445

0771279

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

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

infoschema: fix issue of information schema cache miss cause by schema version gap (#53445) #53583

Merged

13 tasks

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

infoschema: fix issue of information schema cache miss cause by schema version gap (#53445) #53584

Merged

13 tasks

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

This is an automated cherry-pick of pingcap#53445

2653442

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

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

infoschema: fix issue of information schema cache miss cause by schema version gap (#53445) #53585

Merged

13 tasks

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

This is an automated cherry-pick of pingcap#53445

47a5f3e

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

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

infoschema: fix issue of information schema cache miss cause by schema version gap (#53445) #53586

Merged

13 tasks

crazycs520 added a commit to ti-chi-bot/tidb that referenced this pull request May 27, 2024

Revert "This is an automated cherry-pick of pingcap#53445"

6daf8fe

This reverts commit 0771279.

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

infoschema: fix issue of information schema cache miss cause by schem…

3a63d1a

…a version gap (pingcap#53445) close pingcap#53428

crazycs520 added a commit to ti-chi-bot/tidb that referenced this pull request May 27, 2024

infoschema: fix issue of information schema cache miss cause by schem…

07fbf65

…a version gap (pingcap#53445) close pingcap#53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

crazycs520 added a commit to ti-chi-bot/tidb that referenced this pull request May 27, 2024

Revert "This is an automated cherry-pick of pingcap#53445"

87edd82

This reverts commit 2653442.

crazycs520 added a commit to ti-chi-bot/tidb that referenced this pull request May 27, 2024

infoschema: fix issue of information schema cache miss cause by schem…

92548d5

…a version gap (pingcap#53445) close pingcap#53428 Signed-off-by: crazycs520 <crazycs520@gmail.com>

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

infoschema: fix issue of information schema cache miss cause by schem…

5a88497

…a version gap (#53445) (#53586) close #53428

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

infoschema: fix issue of information schema cache miss cause by schem…

4399d4a

…a version gap (#53445) (#53583) close #53428

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

infoschema: fix issue of information schema cache miss cause by schem…

a6ebfa3

…a version gap (#53445) (#53585) close #53428

ti-chi-bot bot pushed a commit that referenced this pull request Jun 5, 2024

infoschema: fix issue of information schema cache miss cause by schem…

8dbe4e5

…a version gap (#53445) (#53584) close #53428

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

infoschema: fix issue of information schema cache miss cause by schema version gap #53445

infoschema: fix issue of information schema cache miss cause by schema version gap #53445

crazycs520 commented May 21, 2024 •

edited

Loading

ti-chi-bot bot commented May 21, 2024

tiprow bot commented May 21, 2024

crazycs520 commented May 21, 2024

codecov bot commented May 21, 2024 •

edited

Loading

wjhuang2016 May 21, 2024

crazycs520 May 21, 2024

crazycs520 commented May 21, 2024

tiprow bot commented May 21, 2024

D3Hunter May 22, 2024

crazycs520 May 22, 2024

tiancaiamao left a comment

ti-chi-bot bot commented May 22, 2024

crazycs520 commented May 23, 2024

crazycs520 commented May 23, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

infoschema: fix issue of information schema cache miss cause by schema version gap #53445

infoschema: fix issue of information schema cache miss cause by schema version gap #53445

Conversation

crazycs520 commented May 21, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

before this PR

This PR

Check List

Release note

ti-chi-bot bot commented May 21, 2024

tiprow bot commented May 21, 2024

crazycs520 commented May 21, 2024

codecov bot commented May 21, 2024 • edited Loading

Codecov Report

wjhuang2016 May 21, 2024

Choose a reason for hiding this comment

crazycs520 May 21, 2024

Choose a reason for hiding this comment

crazycs520 commented May 21, 2024

tiprow bot commented May 21, 2024

D3Hunter May 22, 2024

Choose a reason for hiding this comment

crazycs520 May 22, 2024

Choose a reason for hiding this comment

tiancaiamao left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented May 22, 2024

[LGTM Timeline notifier]

crazycs520 commented May 23, 2024

crazycs520 commented May 23, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

ti-chi-bot commented May 27, 2024

crazycs520 commented May 21, 2024 •

edited

Loading

codecov bot commented May 21, 2024 •

edited

Loading