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.

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

Recursive CTE didn't track memory accurately #54181

Closed
guo-shaoge opened this issue Jun 24, 2024 · 3 comments · Fixed by #54208
Closed

Recursive CTE didn't track memory accurately #54181

guo-shaoge opened this issue Jun 24, 2024 · 3 comments · Fixed by #54208
Assignees
Labels
affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.2 report/customer Customers have encountered this bug. severity/major sig/execution SIG execution type/bug The issue is confirmed as a bug.

Comments

@guo-shaoge
Copy link
Collaborator

guo-shaoge commented Jun 24, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

// Reopen impls Storage Reopen interface.
func (s *StorageRC) Reopen() (err error) {
if err = s.rc.Close(); err != nil {
return err
}
s.iter = 0
s.done = false
s.err = nil
// Create a new RowContainer.
// Because some meta infos in old RowContainer are not resetted.
// Such as memTracker/actionSpill etc. So we just use a new one.
s.rc = chunk.NewRowContainer(s.tp, s.chkSize)
return nil
}

L167 create a new row container after each CTE iteration. But it didn't attach memtracker to session tracker, so the memory control will not work correctly.

Specifically, the memory control for resTbl of CTE is as expected, but after one iteration of the CTE computation, the memory usage of iterInTbl and iterOutTbl cannot be accurately tracked (and they also cannot be spilled to disk) because of above bug.

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your TiDB version? (Required)

@guo-shaoge guo-shaoge added type/bug The issue is confirmed as a bug. sig/execution SIG execution labels Jun 24, 2024
@guo-shaoge guo-shaoge self-assigned this Jun 24, 2024
@guo-shaoge guo-shaoge added affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 affects-7.0 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 affects-7.3 affects-7.4 affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-7.6 affects-8.0 affects-8.1 This bug affects the 8.1.x(LTS) versions. labels Jun 24, 2024
@yibin87
Copy link
Contributor

yibin87 commented Jun 25, 2024

Reproduce case:
drop table if exists tbl_0;
create table tbl_0(a int);
insert into tbl_0 values(100);
insert into tbl_0 values(200);
insert into tbl_0 values(300);
insert into tbl_0 values(400);
set cte_max_recursion_depth = 10000000;
set tidb_mem_quota_query = 10000;
explain analyze with recursive cte_0 (col_10,col_11,col_12,col_13,col_14,col_15,col_16,col_17,col_18) AS ( select 1, 2,3,4,5,6,7,"A", "B" from tbl_0 UNION all select col_10 + 1,col_11 + 1,col_12 + 1 , col_13+1, col_14+1, col_15+1, col_16+1, concat(col_17, "B"), concat(col_18, "C") from cte_0 where col_10 < 19800000 ) select * from cte_0 order by col_18, col_14 limit 4;

The max memory usage tracked is 7-8k bytes. However, the heap profile info shows the CTE operator uses at least 160M.

@JasonWu0506
Copy link

/severity major

@guo-shaoge
Copy link
Collaborator Author

guo-shaoge commented Jun 25, 2024

todo: Also remember to check if memory/cpu is back to normal when this sql is killed!

@seiya-annie seiya-annie added the report/customer Customers have encountered this bug. label Jun 28, 2024
@ti-chi-bot ti-chi-bot bot closed this as completed in 479f4be Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.2 report/customer Customers have encountered this bug. severity/major sig/execution SIG execution type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants