Skip to content

Commit 5c9a5f3

Browse files
AilinKidti-chi-bot
authored andcommitted
This is an automated cherry-pick of pingcap#49192
Signed-off-by: ti-chi-bot <[email protected]>
1 parent b5f3aa0 commit 5c9a5f3

File tree

4 files changed

+53
-12
lines changed

4 files changed

+53
-12
lines changed

pkg/executor/cte.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,24 @@ func (e *CTEExec) Next(ctx context.Context, req *chunk.Chunk) (err error) {
112112

113113
// Close implements the Executor interface.
114114
func (e *CTEExec) Close() (err error) {
115-
e.producer.resTbl.Lock()
116-
if !e.producer.closed {
117-
// closeProducer() only close seedExec and recursiveExec, will not touch resTbl.
118-
// It means you can still read resTbl after call closeProducer().
119-
// You can even call all three functions(openProducer/produce/closeProducer) in CTEExec.Next().
120-
// Separating these three function calls is only to follow the abstraction of the volcano model.
121-
err = e.producer.closeProducer()
122-
}
123-
e.producer.resTbl.Unlock()
115+
func() {
116+
e.producer.resTbl.Lock()
117+
defer e.producer.resTbl.Unlock()
118+
if !e.producer.closed {
119+
failpoint.Inject("mock_cte_exec_panic_avoid_deadlock", func(v failpoint.Value) {
120+
ok := v.(bool)
121+
if ok {
122+
// mock an oom panic, returning ErrMemoryExceedForQuery for error identification in recovery work.
123+
panic(exeerrors.ErrMemoryExceedForQuery)
124+
}
125+
})
126+
// closeProducer() only close seedExec and recursiveExec, will not touch resTbl.
127+
// It means you can still read resTbl after call closeProducer().
128+
// You can even call all three functions(openProducer/produce/closeProducer) in CTEExec.Next().
129+
// Separating these three function calls is only to follow the abstraction of the volcano model.
130+
err = e.producer.closeProducer()
131+
}
132+
}()
124133
if err != nil {
125134
return err
126135
}

pkg/executor/cte_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/stretchr/testify/require"
2828
)
2929

30+
<<<<<<< HEAD
3031
func TestBasicCTE(t *testing.T) {
3132
store := testkit.CreateMockStore(t)
3233

@@ -346,6 +347,37 @@ func TestCTEWithLimit(t *testing.T) {
346347
rows.Check(testkit.Rows("3", "4", "3"))
347348
rows = tk.MustQuery("with recursive cte1(c1) as (select c1 from t1 union all select c1 + 1 from cte1 limit 4 offset 4) select * from cte1;")
348349
rows.Check(testkit.Rows("3", "4", "3", "4"))
350+
=======
351+
func TestCTEIssue49096(t *testing.T) {
352+
store := testkit.CreateMockStore(t)
353+
tk := testkit.NewTestKit(t, store)
354+
355+
tk.MustExec("use test;")
356+
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/executor/mock_cte_exec_panic_avoid_deadlock", "return(true)"))
357+
defer func() {
358+
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/executor/mock_cte_exec_panic_avoid_deadlock"))
359+
}()
360+
insertStr := "insert into t1 values(0)"
361+
rowNum := 10
362+
vals := make([]int, rowNum)
363+
vals[0] = 0
364+
for i := 1; i < rowNum; i++ {
365+
v := rand.Intn(100)
366+
vals[i] = v
367+
insertStr += fmt.Sprintf(", (%d)", v)
368+
}
369+
tk.MustExec("drop table if exists t1, t2;")
370+
tk.MustExec("create table t1(c1 int);")
371+
tk.MustExec("create table t2(c1 int);")
372+
tk.MustExec(insertStr)
373+
// should be insert statement, otherwise it couldn't step int resetCTEStorageMap in handleNoDelay func.
374+
sql := "insert into t2 with cte1 as ( " +
375+
"select c1 from t1) " +
376+
"select c1 from cte1 natural join (select * from cte1 where c1 > 0) cte2 order by c1;"
377+
err := tk.ExecToErr(sql)
378+
require.NotNil(t, err)
379+
require.Equal(t, "[executor:8175]Your query has been cancelled due to exceeding the allowed memory limit for a single SQL query. Please try narrowing your query scope or increase the tidb_mem_quota_query limit and try again.[conn=%d]", err.Error())
380+
>>>>>>> 0c7659c1907 (executor: fix deadlock in dml statement with cte when oom panic action was triggered (#49192))
349381
}
350382

351383
func TestSpillToDisk(t *testing.T) {

pkg/util/cteutil/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ go_library(
1010
"//pkg/util/chunk",
1111
"//pkg/util/disk",
1212
"//pkg/util/memory",
13+
"//pkg/util/syncutil",
1314
"@com_github_pingcap_errors//:errors",
1415
],
1516
)

pkg/util/cteutil/storage.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,12 @@
1515
package cteutil
1616

1717
import (
18-
"sync"
19-
2018
"github.com/pingcap/errors"
2119
"github.com/pingcap/tidb/pkg/types"
2220
"github.com/pingcap/tidb/pkg/util/chunk"
2321
"github.com/pingcap/tidb/pkg/util/disk"
2422
"github.com/pingcap/tidb/pkg/util/memory"
23+
"github.com/pingcap/tidb/pkg/util/syncutil"
2524
)
2625

2726
var _ Storage = &StorageRC{}
@@ -99,7 +98,7 @@ type StorageRC struct {
9998
refCnt int
10099
chkSize int
101100
iter int
102-
mu sync.Mutex
101+
mu syncutil.Mutex
103102
done bool
104103
}
105104

0 commit comments

Comments
 (0)