Skip to content
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.

Sign up for GitHub

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

*: fix auto analyze worker crash when killed by tidb_mem_quota_analyze #39946

Merged
merged 3 commits into from
Dec 15, 2022
Merged

*: fix auto analyze worker crash when killed by tidb_mem_quota_analyze #39946

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5767d36
fix auto analyze worker crash when killed by global memory limit
chrysan Dec 14, 2022
52ee44b
fix
chrysan Dec 15, 2022
b9fd4f2
Merge branch 'master' into fix_auto_analyze_crash
ti-chi-bot Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion executor/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1975,7 +1975,7 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) {
if _, ok := s.(*ast.AnalyzeTableStmt); ok {
sc.InitMemTracker(memory.LabelForAnalyzeMemory, -1)
vars.MemTracker.SetBytesLimit(-1)
vars.MemTracker.AttachTo(GlobalAnalyzeMemoryTracker)
sc.MemTracker.AttachTo(GlobalAnalyzeMemoryTracker)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normal tracker chain is stmt tracker -> session tracker -> global tracker. Here we change it to stmt tracker -> global analyze tracker -> global tracker. I am not sure whether it has some unexpected impact. cc @XuHuaiyu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Line 1996, stmt tracker will attachTo session tracker again.. So the tracker chain will be stmt tracker -> session tracker finally, and all trackers do not attach to GlobalAnalyzeMemoryTracker?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must attach stmt tracker to session tracker, so that global-mem-limit can see manual analyze as top1 consumer from session manager.
While session manager cannot see auto-analyze which is run from a background shared session pool, will handle it in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#39978

} else {
sc.InitMemTracker(memory.LabelForSQLText, -1)
}
Expand Down
5 changes: 5 additions & 0 deletions statistics/handle/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,11 @@ func (h *Handle) getAnalyzeSnapshot() (bool, error) {

// HandleAutoAnalyze analyzes the newly created table or index.
func (h *Handle) HandleAutoAnalyze(is infoschema.InfoSchema) (analyzed bool) {
defer func() {
if r := recover(); r != nil {
logutil.BgLogger().Error("HandleAutoAnalyze panicked", zap.Any("error", r), zap.Stack("stack"))
}
}()
err := h.UpdateSessionVar()
if err != nil {
logutil.BgLogger().Error("[stats] update analyze version for auto analyze session failed", zap.Error(err))
Expand Down