-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
infoschema: add a simply store for DM's SchemaTracker #35954
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/569f7ce78f12442e5fab0c3e7ea7484edd19751a |
Signed-off-by: lance6716 <[email protected]>
It is not related to ddl current code. so I think it can be moved into |
it will be used soon as in #35867 . do you prefer to put infoStore and its usage in one file, or put infoStore in ddl/util.go considering this? |
ddl/dm_schema_tracker.go
Outdated
|
||
// infoStore is a simple structure that stores DBInfo and TableInfo. It's not thread-safe. | ||
type infoStore struct { | ||
lowerCaseTableNames int // same as variable lower_case_table_names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this is not needed at present. Can you remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's needed in ciStr2Key
. In DM's use case, upstream is MySQL so we should set lowerCaseTableNames to MySQL's value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I see lower_case_table_names
is read-only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, in the big PR #35867, I use struct literal to build infoStore in this package. I can write a newInfoStore.
newInfoStore is already here. The usage is
// NewSchemaTracker creates a SchemaTracker. lowerCaseTableNames has the same meaning as MySQL variable lower_case_table_names.
func NewSchemaTracker(lowerCaseTableNames int) SchemaTracker {
return SchemaTracker{
infoStore: newInfoStore(lowerCaseTableNames),
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. DM needs use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PTAL |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 464e829
|
/run-mysql-test |
/run-build |
/run-unit-test |
TiDB MergeCI notify
|
* upstream/master: (77 commits) infoschema: add a simply store for DM's SchemaTracker (pingcap#35954) planner: refactor "access object" related logic and prepare for binary plan (pingcap#35890) telemetry: Initialize telemetry asynchronously (pingcap#35898) sessionctx/variable: convert TiDBLogFileMaxDays and TiDBConfig to ScopeInstance (pingcap#35733) session, sessionctx/variable: ensure sysvars are always in lower case (pingcap#35659) *: add telemetry support for multi-schema change (pingcap#35977) topsql: compress plan instead of dropping it (pingcap#35973) *: use pdqsort to improve sort performance (pingcap#34404) ddl: refine GetDDLInfo function (pingcap#35969) *: Make the code cleaner in session.ExecutePreparedStmt (pingcap#35943) ddl: remove `onDropIndexes` (pingcap#35960) ddl: refine history job code (pingcap#35967) sessionctx,kv,planner: add system variable for fine_grained_shuffle (pingcap#35256) *: change remote cache (pingcap#35968) *: enable gosec (pingcap#35873) plancodec: fix a miss case in conversion (pingcap#35957) ddl: support drop indexes for multi-schema change (pingcap#35883) pdutil: fix unstable test TestPauseSchedulersByKeyRange (pingcap#35949) br: optimize communication with tikv stores. pingcap#31722 (pingcap#35147) txn: rename snapshot related methods in txnManager. (pingcap#35912) ...
What problem does this PR solve?
Issue Number: ref #35933
Problem Summary:
What is changed and how it works?
a simple put/delete store for DBInfo/TableInfo. The unit test can cover all lines of infoStore.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.