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

store/copr: optimize copIterator by avoid start new goroutine #57522

Merged
merged 66 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 60 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
4637ae4
avoid goroutine for when only have 1 cop task
crazycs520 Sep 5, 2024
9708f39
tiny refine
crazycs520 Nov 19, 2024
77c6ff6
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 19, 2024
208d2f6
remove debug log
crazycs520 Nov 19, 2024
e2b5988
fix bug
crazycs520 Nov 20, 2024
2838317
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 20, 2024
0b09caa
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 20, 2024
3ab1142
fix test
crazycs520 Nov 20, 2024
ffdb115
init
crazycs520 Nov 20, 2024
90b1266
refine
crazycs520 Nov 20, 2024
7493230
refine
crazycs520 Nov 20, 2024
9b21d17
refine
crazycs520 Nov 20, 2024
0adfd32
refine
crazycs520 Nov 20, 2024
d42989f
fix test
crazycs520 Nov 20, 2024
986275f
fix test
crazycs520 Nov 20, 2024
5bd2cab
skip test
crazycs520 Nov 21, 2024
4dd1749
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 21, 2024
3936e38
fix test
crazycs520 Nov 21, 2024
231f80b
refine
crazycs520 Nov 21, 2024
328f78d
refactor
crazycs520 Nov 21, 2024
8e23f99
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 21, 2024
2525b37
fix test
crazycs520 Nov 22, 2024
74c705c
refactor
crazycs520 Nov 22, 2024
d75b47c
fix test
crazycs520 Nov 22, 2024
62659f1
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 22, 2024
cc5b429
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 25, 2024
05c68e5
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 26, 2024
e3cc04b
fix test
crazycs520 Nov 26, 2024
b2e5621
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 27, 2024
640ba95
tiny fix
crazycs520 Nov 28, 2024
6dd743d
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 29, 2024
814379a
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Nov 29, 2024
cd8962d
refine
crazycs520 Dec 2, 2024
d9dd69f
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 2, 2024
b218c8e
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 3, 2024
f6d6b23
fix test
crazycs520 Dec 4, 2024
e103ffa
address comment
crazycs520 Dec 9, 2024
9c56def
fix for batch_tasks
crazycs520 Dec 9, 2024
b88709a
address comment
crazycs520 Dec 9, 2024
9bf22a9
fix test
crazycs520 Dec 9, 2024
0243265
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 9, 2024
f88c931
address comment
crazycs520 Dec 9, 2024
cfbf4f9
refine comment
crazycs520 Dec 9, 2024
632f81f
address comment
crazycs520 Dec 10, 2024
3e516f3
refactor
crazycs520 Dec 10, 2024
a832de9
refine
crazycs520 Dec 11, 2024
05253f4
refine
crazycs520 Dec 11, 2024
c652cf9
refine
crazycs520 Dec 11, 2024
0854b6e
remove lite worker
crazycs520 Dec 11, 2024
5324833
tiny fix
crazycs520 Dec 11, 2024
2e6c5c7
fix ci lint
crazycs520 Dec 11, 2024
c4cf2d5
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 11, 2024
c2e8f01
refine
crazycs520 Dec 11, 2024
0fc1818
refine
crazycs520 Dec 11, 2024
e382b9c
add test
crazycs520 Dec 11, 2024
53010ee
fix ci lint
crazycs520 Dec 11, 2024
0339fd2
Revert "remove lite worker"
crazycs520 Dec 11, 2024
a221597
refine comment
crazycs520 Dec 11, 2024
03f245a
fix bug
crazycs520 Dec 11, 2024
d4f5b0b
fix
crazycs520 Dec 11, 2024
3a6f46e
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 11, 2024
6355e0d
fix bug
crazycs520 Dec 12, 2024
307eeba
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 12, 2024
a3c7e53
add test case
crazycs520 Dec 12, 2024
e3f13d1
refine
crazycs520 Dec 12, 2024
fdeff5b
Merge branch 'master' of https://github.com/pingcap/tidb into opt-cop…
crazycs520 Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/distsql/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ type DistSQLContext struct {
SessionAlias string

ExecDetails *execdetails.SyncExecDetails

// Only one cop-reader can use lite worker. Using lite-worker in multiple readers will affect the concurrent execution of readers.
TryCopLiteWorker uint32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined that the TryCopLiteWorker is a request level option.

Like an index lookup with 1 row result set. If TryCopLiteWorker is a session level option, it's only enabled once in index scan, and in table lookup, the atomic.CompareAndSwapUint32(tryCopLiteWorker, 0, 1) will fail and execute with multi-goroutine model.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that make TryCopLiteWorker in request level option may have similar problem in this comment, there is a known case and I have added test for it. And I'm not sure if there are other cases like this, so I prefer keep TryCopLiteWorker to be statement level.

As index lookup with 1 row result set, you are right, but because of the above problems, I don't have a good way to fix this.

}

// AppendWarning appends the warning to the warning handler.
Expand Down
1 change: 1 addition & 0 deletions pkg/distsql/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func TestContextDetach(t *testing.T) {
ReplicaClosestReadThreshold: 1,
ConnectionID: 1,
SessionAlias: "c",
TryCopLiteWorker: 1,
}

obj.AppendWarning(errors.New("test warning"))
Expand Down
1 change: 1 addition & 0 deletions pkg/distsql/distsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func Select(ctx context.Context, dctx *distsqlctx.DistSQLContext, kvReq *kv.Requ
EnabledRateLimitAction: enabledRateLimitAction,
EventCb: eventCb,
EnableCollectExecutionInfo: config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Load(),
TryCopLiteWorker: &dctx.TryCopLiteWorker,
}

if kvReq.StoreType == kv.TiFlash {
Expand Down
1 change: 1 addition & 0 deletions pkg/kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ type ClientSendOption struct {
EnableCollectExecutionInfo bool
TiFlashReplicaRead tiflash.ReplicaRead
AppendWarning func(warn error)
TryCopLiteWorker *uint32
}

// ReqTypes.
Expand Down
4 changes: 3 additions & 1 deletion pkg/store/copr/copr_test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ go_test(
"main_test.go",
],
flaky = True,
shard_count = 3,
shard_count = 4,
deps = [
"//pkg/config",
"//pkg/kv",
"//pkg/resourcegroup/runaway",
"//pkg/store/copr",
"//pkg/store/mockstore",
"//pkg/testkit",
"//pkg/testkit/testmain",
"//pkg/testkit/testsetup",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_pingcap_kvproto//pkg/meta_storagepb",
"@com_github_pingcap_kvproto//pkg/resource_manager",
"@com_github_stretchr_testify//require",
Expand Down
21 changes: 21 additions & 0 deletions pkg/store/copr/copr_test/coprocessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,18 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"testing"
"time"

"github.com/pingcap/failpoint"
"github.com/pingcap/kvproto/pkg/meta_storagepb"
rmpb "github.com/pingcap/kvproto/pkg/resource_manager"
"github.com/pingcap/tidb/pkg/kv"
"github.com/pingcap/tidb/pkg/resourcegroup/runaway"
"github.com/pingcap/tidb/pkg/store/copr"
"github.com/pingcap/tidb/pkg/store/mockstore"
"github.com/pingcap/tidb/pkg/testkit"
"github.com/stretchr/testify/require"
"github.com/tikv/client-go/v2/testutils"
pd "github.com/tikv/pd/client"
Expand Down Expand Up @@ -287,3 +290,21 @@ func TestBuildCopIteratorWithRunawayChecker(t *testing.T) {
require.Equal(t, concurrency, 1)
require.Equal(t, smallTaskConcurrency, 0)
}

func TestQueryWithConcurrentSmallCop(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
// Test for https://github.com/pingcap/tidb/pull/57522#discussion_r1875515863
tk.MustExec("create table t1 (id int key, b int, c int, index idx_b(b)) partition by hash(id) partitions 10;")
for i := 0; i < 10; i++ {
tk.MustExec(fmt.Sprintf("insert into t1 values (%v, %v, %v)", i, i, i))
}

tk.MustExec("set @@tidb_distsql_scan_concurrency=15")
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/store/mockstore/unistore/unistoreRPCSlowCop", `return(200)`))
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
tk.MustQueryWithContext(ctx, "select sum(c) from t1 use index (idx_b) where b < 10;")
cancel()
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/mockstore/unistore/unistoreRPCSlowCop"))
}
Loading