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) (#48475)

close #48212
  • Loading branch information
ti-chi-bot authored Nov 13, 2023
1 parent 30498cc commit ba7fe19
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 @@ -699,3 +699,25 @@ func TestCoprocessorBatchByStore(t *testing.T) {
}
}
}

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 @@ -1637,7 +1637,14 @@ func (worker *copIteratorWorker) handleCopCache(task *copTask, resp *copResponse
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
}
return nil
}
copr_metrics.CoprCacheCounterMiss.Add(1)
Expand Down

0 comments on commit ba7fe19

Please sign in to comment.