*: add statistic lock/unlock function #38387
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. |
ti-chi-bot
added
do-not-merge/invalid-title
release-note-none
|
|
ti-chi-bot
added
release-note
pingandb
force-pushed
the
statistic
branch
7 times, most recently
from
3135d54
to
2678a7b
Compare
|
/run-unit-test |
2 similar comments
|
/run-unit-test |
|
/run-unit-test |
pingandb
force-pushed
the
statistic
branch
3 times, most recently
from
be1e602
to
bb32af0
Compare
|
/run-unit-test |
|
/run-all-tests |
ti-chi-bot
removed
the
needs-rebase
|
/run-all-tests |
ti-chi-bot
added
the
needs-rebase
ti-chi-bot
added
needs-rebase
| tableID = task.idxIncrementalExec.tableID | ||
| } | ||
| // skip locked tables | ||
| if !statsHandle.IsTableLocked(tableID.TableID) { |
There was a problem hiding this comment.
Maybe we can use warnings to remind users that the table is locked.
statistics/handle/handle.go
Outdated
| } | ||
|
|
||
| if len(tids) > 1 { | ||
| err = finishTransaction(ctx, exec, err) |
There was a problem hiding this comment.
Maybe it is better to defer finishTransaction right after begin begin pessimistic.
There was a problem hiding this comment.
if finishTransaction get failed, 'mysql.stats_table_locked' won't be updated, and "h.tableLocked = tableLocked" won't be execute, so h.tableLocked is consistent with table.
if change to defer finishTransaction , if finishTransaction get failed, table won't be updated but h.tableLocked will be updated, data in table and memory is inconsistent.
|
@pingandb Please solve the conflicting. |
statistics/handle/handle.go
Outdated
| } | ||
|
|
||
| // GetTableLockedAndClear for unit test only | ||
| func (h *Handle) GetTableLockedAndClear() []int64 { |
There was a problem hiding this comment.
Add ForTest to make its scope clearer
ti-chi-bot
removed
the
needs-rebase
statistics/handle/handle.go
Outdated
|
|
||
| exec := h.mu.ctx.(sqlexec.SQLExecutor) | ||
|
|
||
| if len(tids) > 1 { |
There was a problem hiding this comment.
Now we also have to consider len(pids). Maybe we can all use begin pessimistic for simplicity?
statistics/handle/handle.go
Outdated
| ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnStats) | ||
|
|
||
| exec := h.mu.ctx.(sqlexec.SQLExecutor) | ||
| if len(tids) > 1 { |
ti-chi-bot
added
status/LGT2
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 1d7fdc6
|
ti-chi-bot
added
the
status/can-merge
TiDB MergeCI notify🔴 Bad News! New failing [3] after this pr merged.
|
What problem does this PR solve?
Issue Number: close #xxx
Problem Summary:
Sometime peformance drop due to the explain change that caused by statistic info change after data changes in table.
So we want to introduce table statistic lock/unlock function to avoid unexpected peformance drop by statistic changes.
What is changed and how it works?
The main idea is adding new table mysql.stats_table_locked to record which table is locked, and when doing manual/auto table analyze, check whether table is locked, if locked, skip analyse. And I also filter the locked table in session stat collector operations, that session stat collector won't collect the statistic data of locked table.
Add SQL syntax 'lock/unlock stats xxx(table name)' to add/remove table name to/from mysql.stats_table_locked.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.