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

close #48212
  • Loading branch information
ti-chi-bot authored Jan 12, 2024
1 parent 0ba14f4 commit 8a15fe4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
22 changes: 22 additions & 0 deletions pkg/executor/distsql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,3 +698,25 @@ func TestIndexLookUpWithSelectForUpdateOnPartitionTable(t *testing.T) {
tk.MustHavePlan("select b from t use index(k) where b > 2 order by b limit 1 for update", "IndexLookUp")
tk.MustQuery("select b from t use index(k) where b > 2 order by b limit 1 for update").Check(testkit.Rows("3"))
}

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/pkg/store/mockstore/unistore/cophandler/mockCopCacheInUnistore", `return(123)`))
defer func() {
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/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 pkg/store/copr/coprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1685,7 +1685,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 8a15fe4

Please sign in to comment.