-
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
pitr: add ingest recorder to repair indexes #41670
Conversation
Signed-off-by: Leavrth <jianjun.liao@outlook.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: Leavrth <jianjun.liao@outlook.com>
br/pkg/restore/client.go
Outdated
const ( | ||
alterTableDropIndexFormat = "ALTER TABLE `%s`.`%s` DROP INDEX `%s`;" | ||
alterTableAddIndexFormat = "ALTER TABLE `%s`.`%s` ADD INDEX `%s`(%s);" | ||
alterTableAddPrimaryFormat = "ALTER TABLE `%s`.`%s` ADD PRIMARY KEY (%s)" |
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.
here, currently, do not support clusterd PK online changed. also should care able add unique index.
var addSQL string | ||
if info.IsPrimary { | ||
addSQL = fmt.Sprintf(alterTableAddPrimaryFormat, info.SchemaName, info.TableName, info.ColumnList) | ||
} else { |
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.
Add generate uk statement
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
br/pkg/restore/client.go
Outdated
log.Debug("repair ingest sql", zap.String("drop", dropSQL)) | ||
if err := rc.db.se.Execute(ctx, dropSQL); err != nil { | ||
return errors.Trace(err) | ||
} | ||
log.Debug("repair ingest sql", zap.String("add", addSQL)) | ||
if err := rc.db.se.ExecuteInternal(ctx, addSQL, info.IndexInfo.Comment); err != nil { | ||
return errors.Trace(err) | ||
} |
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.
Is the debug log left here on purpose? If not, please remove them.
Why does dropSQL
use Execute()
and addSQL()
use ExecuteInternal
?
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.
Because the Comment
has some escape characters such as "
and \n
, so here use ExecuteInter
to send with args instead of plaintext.
The function pattern is as follow:
Execute(context, sql string)
ExecuteInternal(context, sql string, args...)
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.
done, both use ExecuteInternal
.
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
br/pkg/restore/client.go
Outdated
) | ||
|
||
if info.IsPrimary { | ||
addSQL = fmt.Sprintf(alterTableAddPrimaryFormat, info.SchemaName, info.TableName, info.ColumnList) |
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.
Should we escape here? Perhaps some guys like to name their indices like(How absurd!):
CREATE INDEX `idx``; DROP DATABASE business; --`(some_row);
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 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.
done. test updated
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
br/pkg/restore/client.go
Outdated
// RangeFilterFromIngestRecorder rewrites the table id of items in the ingestRecorder | ||
// TODO: need to implement the range filter out feature | ||
func (rc *Client) RangeFilterFromIngestRecorder(recorder *ingestrec.IngestRecorder, rewriteRules map[int64]*RewriteRules) error { | ||
filter := rtree.NewRangeTree() |
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.
What is the filter
used for?
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 1b57703
|
/hold |
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
/retest |
1 similar comment
/retest |
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 for the PB part
/retest |
1 similar comment
/retest |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 739bef9
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
/cherry-pick release-6.5 |
@BornChanger: new pull request created to branch 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 ti-community-infra/tichi repository. |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
…ingcap#46418)" This reverts commit a1af97e.
What problem does this PR solve?
Issue Number: close #41668 ref #38045
Problem Summary:
PiTR is not compatible with accelerated indexing
What is changed and how it works?
Check List
Tests
upstream SQL
restore repair ingest index SQL from BR log
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.