Skip to content

Commit

Permalink
copr: fix copr cache panic when tidb_enable_collect_execution_info
Browse files Browse the repository at this point in the history
…is off (#48340) (#48474)

close #48212
  • Loading branch information
ti-chi-bot authored Nov 13, 2023
1 parent cf8dbc2 commit 1a18c04
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
22 changes: 22 additions & 0 deletions executor/distsql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,3 +640,25 @@ func TestCoprocessorPagingReqKeyRangeSorted(t *testing.T) {
tk.MustExec(`set @a=0x61219F79C90D3541F70E, @b=5501707547099269248, @c=0xEC43EFD30131DEA2CB8B, @d="呣丼蒢咿卻鹻铴础湜僂頃dž縍套衞陀碵碼幓9", @e="鹹楞睕堚尛鉌翡佾搁紟精廬姆燵藝潐楻翇慸嵊";`)
tk.MustExec(`execute stmt using @a,@b,@c,@d,@e;`)
}

func TestCoprCacheWithoutExecutionInfo(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk1 := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(id int)")
tk.MustExec("insert into t values(1), (2), (3)")

require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/mockstore/unistore/cophandler/mockCopCacheInUnistore", `return(123)`))
defer func() {
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/mockstore/unistore/cophandler/mockCopCacheInUnistore"))
}()

defer tk.MustExec("set @@tidb_enable_collect_execution_info=1")
ctx := context.WithValue(context.Background(), "CheckSelectRequestHook", func(_ *kv.Request) {
tk1.MustExec("set @@tidb_enable_collect_execution_info=0")
})
tk.MustQuery("select * from t").Check(testkit.Rows("1", "2", "3"))
tk.MustQueryWithContext(ctx, "select * from t").Check(testkit.Rows("1", "2", "3"))
}
9 changes: 8 additions & 1 deletion store/copr/coprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,14 @@ func (worker *copIteratorWorker) handleCopResponse(bo *Backoffer, rpcCtx *tikv.R
resp.pbResp.Range = nil
}
}
resp.detail.CoprCacheHit = true
// `worker.enableCollectExecutionInfo` is loaded from the instance's config. Because it's not related to the request,
// the cache key can be same when `worker.enableCollectExecutionInfo` is true or false.
// When `worker.enableCollectExecutionInfo` is false, the `resp.detail` is nil, and hit cache is still possible.
// Check `resp.detail` to avoid panic.
// Details: https://github.com/pingcap/tidb/issues/48212
if resp.detail != nil {
resp.detail.CoprCacheHit = true
}
} else {
coprCacheCounterMiss.Add(1)
// Cache not hit or cache hit but not valid: update the cache if the response can be cached.
Expand Down

0 comments on commit 1a18c04

Please sign in to comment.