-
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
*: add a config to enable write statements to read on TiFlash #37516
Conversation
Signed-off-by: gengliqi <gengliqiii@gmail.com>
Signed-off-by: gengliqi <gengliqiii@gmail.com>
[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. |
Signed-off-by: gengliqi <gengliqiii@gmail.com>
sessionctx/variable/tidb_vars.go
Outdated
// TiFlashReadForWriteStmt indicates whether to enable write stmts to read on TiFlash. | ||
TiFlashReadForWriteStmt = "tiflash_read_for_write_stmt" |
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.
There are two conventions with sysvars:
- Most variable names start with tidb_XX.
- Boolean options usually use the name tidb_enable_XX
Is it possible to name it something like tidb_enable_tiflash_for_dml
?
(unfortunately there are exceptions to both of these conventions, but they are mostly followed.)
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.
select
is considered to be part of DML(strictly speaking, it's DQL). What I want to express is that the read part in insert/update/delete
SQL can be pushed down to TiFlash.
How about tidb_enable_tiflash_for_all_dml
or tidb_enable_tiflash_read_for_write_stmt
?
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.
agree with https://github.com/pingcap/tidb/pull/37516/files#r959628864, others LGTM
Signed-off-by: gengliqi <gengliqiii@gmail.com>
Signed-off-by: gengliqi <gengliqiii@gmail.com>
Signed-off-by: gengliqi <gengliqiii@gmail.com>
/merge |
/run-mysql-test |
/merge |
/run-mysql-test tidb-test=pr/1957 |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 92bd9e6
|
/run-mysql-test tidb-test=pr/1957 |
/run-mysql-test tidb-test=pr/1957 |
Signed-off-by: gengliqi gengliqiii@gmail.com
What problem does this PR solve?
Issue Number: close #37515
Problem Summary:
What is changed and how it works?
Add a session config
tidb_enable_tiflash_read_for_write_stmt
.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.