infoschema: tend to cache the old key in caching strategy for infoschema v2 #54501

tiancaiamao · 2024-07-08T10:00:51Z

What problem does this PR solve?

Issue Number: close #54500
ref #50959

Problem Summary:

What changed and how does it work?

The bug is caused by the old caching strategy.
When we check the schema version from the btree, we get the old cache key.
When we update the cache, we use the latest schema version as the cache key.
It means when the old cache key is evicted, we will always cache miss and the refill does not help.

In this commit, I change the caching strategy tending to cache the old schema version.

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

tiprow · 2024-07-08T10:01:08Z

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

codecov · 2024-07-08T10:25:07Z

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 56.2954%. Comparing base (749d303) to head (6428767).

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #54501         +/-   ##
=================================================
- Coverage   74.8681%   56.2954%   -18.5727%     
=================================================
  Files          1549       1665        +116     
  Lines        362122     604095     +241973     
=================================================
+ Hits         271114     340078      +68964     
- Misses        71466     240690     +169224     
- Partials      19542      23327       +3785

Flag	Coverage Δ
integration	`37.0036% <14.2857%> (?)`
unit	`71.7947% <85.7142%> (-2.0010%)`	⬇️

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

Components	Coverage Δ
dumpling	`52.9656% <ø> (-2.2339%)`	⬇️
parser	`∅ <ø> (∅)`
br	`52.5794% <ø> (+4.9011%)`	⬆️

GMHDBJD

LGTM

tiancaiamao · 2024-07-08T12:23:44Z

/retest-required

tiprow · 2024-07-08T12:24:05Z

@tiancaiamao: 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

rest lgtm

pkg/infoschema/infoschema_v2.go

D3Hunter · 2024-07-09T04:00:54Z

pkg/infoschema/infoschema_v2.go

@@ -444,6 +440,17 @@ func isSpecialDB(dbName string) bool {
 		dbName == util.MetricSchemaName.L
 }

+// EvictTable is exported for testing only.
+func (is *infoschemaV2) EvictTable(schema, tbl string) {


maybe implement it in test file of the same package, see

tidb/pkg/disttask/framework/scheduler/main_test.go

Lines 25 to 32 in ef0a3eb

func (sm *Manager) GetRunningTaskCnt() int {

return sm.getSchedulerCount()

}

// DelRunningTask implements Scheduler.DelRunningTask interface.

func (sm *Manager) DelRunningTask(id int64) {

sm.delScheduler(id)

}

Some other parts may also need this functionality and it's in some other packages.
https://github.com/pingcap/tidb/pull/54479/files#diff-cabf59f012600c8b513d4eb03385f48e3fdd9b8ac3f13c175133d40c9354722bR279

pkg/infoschema/test/infoschemav2test/v2_test.go

ti-chi-bot · 2024-07-09T07:15:19Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~pkg/infoschema/OWNERS~~ [D3Hunter,GMHDBJD]

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-07-09T07:15:23Z

[LGTM Timeline notifier]

Timeline:

2024-07-08 12:13:54.494152918 +0000 UTC m=+273331.729387030: ☑️ agreed by GMHDBJD.
2024-07-09 07:15:22.506371333 +0000 UTC m=+341819.741605438: ☑️ agreed by D3Hunter.

tiancaiamao · 2024-07-09T08:47:46Z

/test unit-test

tiprow · 2024-07-09T08:48:10Z

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

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

tiancaiamao · 2024-07-09T15:55:59Z

/test mysql-test
/test check-dev2

tiprow · 2024-07-09T15:56:23Z

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

In response to this:

/test mysql-test
/test check-dev2

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.

tiancaiamao · 2024-07-10T06:40:56Z

/test mysql-test
/test check-dev2

tiprow · 2024-07-10T06:41:23Z

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

In response to this:

/test mysql-test
/test check-dev2

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.

tiancaiamao · 2024-07-10T07:00:24Z

/test mysql-test
/test check-dev2

tiprow · 2024-07-10T07:00:45Z

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

In response to this:

/test mysql-test
/test check-dev2

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.

tiancaiamao · 2024-07-10T07:22:05Z

/ok-to-test

tiancaiamao · 2024-07-10T07:41:53Z

Need to wait this PR to merge first #54282
The mysql-test has been merged, and that one is merging, so in the meantime the CI for master branch would fail temporarily

tiancaiamao · 2024-07-10T13:51:06Z

/test mysql-test
/test check-dev2

tiprow · 2024-07-10T13:51:28Z

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

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test mysql-test
/test check-dev2

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.

tiancaiamao · 2024-07-11T01:09:03Z

/test mysql-test

tiprow · 2024-07-11T01:09:24Z

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

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test mysql-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-sigs/prow repository.

tiancaiamao · 2024-07-11T02:36:10Z

/test check-dev2

tiprow · 2024-07-11T02:36:31Z

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

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test check-dev2

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.

tiancaiamao · 2024-07-11T04:21:21Z

/retest

tiancaiamao added 3 commits July 3, 2024 09:51

infoschema: correct some comments for the code which are out-dated

a66cbc2

Merge branch 'master' into correct-comments

468e24c

handle the reload

2ee490b

ti-chi-bot bot added release-note-none do-not-merge/needs-triage-completed labels Jul 8, 2024

tiancaiamao requested a review from GMHDBJD July 8, 2024 10:00

ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 8, 2024

tiancaiamao requested a review from D3Hunter July 8, 2024 10:01

GMHDBJD approved these changes Jul 8, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm labels Jul 8, 2024

D3Hunter reviewed Jul 9, 2024

View reviewed changes

address comment

f29e10f

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

D3Hunter approved these changes Jul 9, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm labels Jul 9, 2024

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2024

Merge branch 'master' into cache-strategy

f394b10

ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2024

make bazel_prepare

ffb72d1

ti-chi-bot bot added the ok-to-test label Jul 10, 2024

Merge branch 'master' into cache-strategy

6428767

ti-chi-bot bot merged commit 05c8119 into pingcap:master Jul 11, 2024
23 checks passed

tiancaiamao deleted the cache-strategy branch July 11, 2024 05:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

infoschema: tend to cache the old key in caching strategy for infoschema v2 #54501

infoschema: tend to cache the old key in caching strategy for infoschema v2 #54501

tiancaiamao commented Jul 8, 2024

tiprow bot commented Jul 8, 2024

codecov bot commented Jul 8, 2024 •

edited

Loading

GMHDBJD left a comment

tiancaiamao commented Jul 8, 2024

tiprow bot commented Jul 8, 2024

D3Hunter left a comment

D3Hunter Jul 9, 2024

tiancaiamao Jul 9, 2024

ti-chi-bot bot commented Jul 9, 2024

ti-chi-bot bot commented Jul 9, 2024

tiancaiamao commented Jul 9, 2024

tiprow bot commented Jul 9, 2024

tiancaiamao commented Jul 9, 2024

tiprow bot commented Jul 9, 2024

tiancaiamao commented Jul 10, 2024

tiprow bot commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiprow bot commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiprow bot commented Jul 10, 2024

tiancaiamao commented Jul 11, 2024

tiprow bot commented Jul 11, 2024

tiancaiamao commented Jul 11, 2024

tiprow bot commented Jul 11, 2024

tiancaiamao commented Jul 11, 2024

	func (sm *Manager) GetRunningTaskCnt() int {
	return sm.getSchedulerCount()
	}

	// DelRunningTask implements Scheduler.DelRunningTask interface.
	func (sm *Manager) DelRunningTask(id int64) {
	sm.delScheduler(id)
	}

infoschema: tend to cache the old key in caching strategy for infoschema v2 #54501

infoschema: tend to cache the old key in caching strategy for infoschema v2 #54501

Conversation

tiancaiamao commented Jul 8, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Jul 8, 2024

codecov bot commented Jul 8, 2024 • edited Loading

Codecov Report

GMHDBJD left a comment

Choose a reason for hiding this comment

tiancaiamao commented Jul 8, 2024

tiprow bot commented Jul 8, 2024

D3Hunter left a comment

Choose a reason for hiding this comment

D3Hunter Jul 9, 2024

Choose a reason for hiding this comment

tiancaiamao Jul 9, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jul 9, 2024

ti-chi-bot bot commented Jul 9, 2024

[LGTM Timeline notifier]

tiancaiamao commented Jul 9, 2024

tiprow bot commented Jul 9, 2024

tiancaiamao commented Jul 9, 2024

tiprow bot commented Jul 9, 2024

tiancaiamao commented Jul 10, 2024

tiprow bot commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiprow bot commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiancaiamao commented Jul 10, 2024

tiprow bot commented Jul 10, 2024

tiancaiamao commented Jul 11, 2024

tiprow bot commented Jul 11, 2024

tiancaiamao commented Jul 11, 2024

tiprow bot commented Jul 11, 2024

tiancaiamao commented Jul 11, 2024

codecov bot commented Jul 8, 2024 •

edited

Loading