domain,infoschema: fix 'show tables' after snapshot read #55856
Conversation
ti-chi-bot
bot
added
release-note-none
|
Maybe it will also fix #55486 but maybe not. |
|
Maybe it will also fix #55806 but maybe not. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #55856 +/- ##
=================================================
- Coverage 72.9093% 56.8786% -16.0307%
=================================================
Files 1604 1755 +151
Lines 446718 630451 +183733
=================================================
+ Hits 325699 358592 +32893
- Misses 100964 247179 +146215
- Partials 20055 24680 +4625
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/retest |
|
@tiancaiamao: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
| // Why? imagine that the current schema version is [103 104 105 ...] | ||
| // Then a snapshot read require infoschem version 53, and it's added | ||
| // Now the history becomes [53, ... 103, 104, 105 ...] | ||
| // Then if a query ask for version 74, we'll mistakenly use 53! |
There was a problem hiding this comment.
And now the query for 74 will fail? Is this expected?
There was a problem hiding this comment.
Of course not.
Not we get two kinds of infoschema, with non-snapshot one [ ... 103, 104, 105 ...]
and snapshot infoschema contains no history like [53] [74] etc
| @@ -533,3 +533,39 @@ func TestSnapshotInfoschemaReader(t *testing.T) { | |||
| sql = fmt.Sprintf("select * from INFORMATION_SCHEMA.TABLES as of timestamp '%s' where table_schema = 'issue55827'", timeStr) | |||
| tk.MustQuery(sql).Check(testkit.Rows()) | |||
| } | |||
|
|
|||
|
|
|||
| func TestIssue55835(t *testing.T) { | |||
There was a problem hiding this comment.
The UT can pass without this fix
There was a problem hiding this comment.
The problem is, when we switch v1/v2 back and forth, the history is not cleared.
So v1/v2 switch can't reproduce the tidb restart scenario.
During adding this test, I did find something wrong: when modify @@tidb_schema_cache_size, without a new DDL operation, v1/v2 switch does not happen, that's not expected.
I confirmed this change can fix the bug and tested manually following the steps described in the issue.
|
PTAL @lance6716 @wjhuang2016 |
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
|
/hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
|
/lgtm |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lance6716, wjhuang2016 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:
Approvers can indicate their approval by writing |
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
|
/test check-dev |
|
@tiancaiamao: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
What problem does this PR solve?
Issue Number: close #55835
Problem Summary:
What changed and how does it work?
loadInfoschema for snapshot infoschema should not be added to schema history.
Check List
Tests
It requires restart of tidb, follow the steps in the issue.
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.