From 50294bb8d72dd3cd07ffb3c989838343956bd589 Mon Sep 17 00:00:00 2001 From: Shenghui Wu <793703860@qq.com> Date: Wed, 13 Nov 2024 14:46:32 +0800 Subject: [PATCH 01/13] executor: support left outer semi join for hash join v2 (#57053) ref pingcap/tidb#53127 handle key-too-large error from MemBuffer Signed-off-by: you06 test MemBuffer's oversize error to tidb error Signed-off-by: you06 update errdoc Signed-off-by: you06 --- errors.toml | 5 +++++ pkg/errno/errcode.go | 1 + pkg/errno/errname.go | 4 +++- pkg/kv/error.go | 2 ++ pkg/store/driver/error/error.go | 5 +++++ pkg/store/driver/error/error_test.go | 14 ++++++++++++++ pkg/store/driver/txn/BUILD.bazel | 2 +- pkg/store/driver/txn/driver_test.go | 4 ++++ 8 files changed, 35 insertions(+), 2 deletions(-) diff --git a/errors.toml b/errors.toml index 1b79985c72a08..d771f0420a5b1 100644 --- a/errors.toml +++ b/errors.toml @@ -2166,6 +2166,11 @@ error = ''' not implemented ''' +["kv:8178"] +error = ''' +key is too large, the size of given key is %d +''' + ["kv:9007"] error = ''' Write conflict, txnStartTS=%d, conflictStartTS=%d, conflictCommitTS=%d, key=%s%s%s%s, reason=%s [try again later] diff --git a/pkg/errno/errcode.go b/pkg/errno/errcode.go index 17fd2cc63ef86..044eda4acf284 100644 --- a/pkg/errno/errcode.go +++ b/pkg/errno/errcode.go @@ -1085,6 +1085,7 @@ const ( ErrMemoryExceedForQuery = 8175 ErrMemoryExceedForInstance = 8176 ErrDeleteNotFoundColumn = 8177 + ErrKeyTooLarge = 8178 // Error codes used by TiDB ddl package ErrUnsupportedDDLOperation = 8200 diff --git a/pkg/errno/errname.go b/pkg/errno/errname.go index 3259373293505..eedd8c562bf4f 100644 --- a/pkg/errno/errname.go +++ b/pkg/errno/errname.go @@ -1078,7 +1078,9 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{ ErrMemoryExceedForQuery: mysql.Message("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]", nil), ErrMemoryExceedForInstance: mysql.Message("Your query has been cancelled due to exceeding the allowed memory limit for the tidb-server instance and this query is currently using the most memory. Please try narrowing your query scope or increase the tidb_server_memory_limit and try again.[conn=%d]", nil), ErrDeleteNotFoundColumn: mysql.Message("Delete can not find column %s for table %s", nil), - ErrHTTPServiceError: mysql.Message("HTTP request failed with status %s", nil), + ErrKeyTooLarge: mysql.Message("key is too large, the size of given key is %d", nil), + + ErrHTTPServiceError: mysql.Message("HTTP request failed with status %s", nil), ErrWarnOptimizerHintInvalidInteger: mysql.Message("integer value is out of range in '%s'", nil), ErrWarnOptimizerHintUnsupportedHint: mysql.Message("Optimizer hint %s is not supported by TiDB and is ignored", nil), diff --git a/pkg/kv/error.go b/pkg/kv/error.go index c8e1bc77a43b1..b09326477bee3 100644 --- a/pkg/kv/error.go +++ b/pkg/kv/error.go @@ -47,6 +47,8 @@ var ( ErrTxnTooLarge = dbterror.ClassKV.NewStd(mysql.ErrTxnTooLarge) // ErrEntryTooLarge is the error when a key value entry is too large. ErrEntryTooLarge = dbterror.ClassKV.NewStd(mysql.ErrEntryTooLarge) + // ErrKeyTooLarge is the error when a key is too large to be handled by MemBuffer. + ErrKeyTooLarge = dbterror.ClassKV.NewStd(mysql.ErrKeyTooLarge) // ErrKeyExists returns when key is already exist. Caller should try to use // GenKeyExistsErr to generate this error for correct format. ErrKeyExists = dbterror.ClassKV.NewStd(mysql.ErrDupEntry) diff --git a/pkg/store/driver/error/error.go b/pkg/store/driver/error/error.go index 914b02a2c76a2..16ac48af5d0b0 100644 --- a/pkg/store/driver/error/error.go +++ b/pkg/store/driver/error/error.go @@ -101,6 +101,11 @@ func ToTiDBErr(err error) error { return kv.ErrEntryTooLarge.GenWithStackByArgs(entryTooLarge.Limit, entryTooLarge.Size) } + var keyTooLarge *tikverr.ErrKeyTooLarge + if stderrs.As(err, &keyTooLarge) { + return kv.ErrKeyTooLarge.GenWithStackByArgs(keyTooLarge.KeySize) + } + if stderrs.Is(err, tikverr.ErrInvalidTxn) { return kv.ErrInvalidTxn } diff --git a/pkg/store/driver/error/error_test.go b/pkg/store/driver/error/error_test.go index b6d3a288d7edb..fd84bc4290241 100644 --- a/pkg/store/driver/error/error_test.go +++ b/pkg/store/driver/error/error_test.go @@ -15,6 +15,7 @@ package error //nolint: predeclared import ( + "math" "testing" "github.com/pingcap/errors" @@ -52,3 +53,16 @@ func TestConvertError(t *testing.T) { assert.True(t, errors.ErrorEqual(tidbErr, terror.ErrResultUndetermined)) } } + +func TestMemBufferOversizeError(t *testing.T) { + err2str := map[error]string{ + &tikverr.ErrTxnTooLarge{Size: 100}: "Transaction is too large, size: 100", + &tikverr.ErrEntryTooLarge{Limit: 10, Size: 20}: "entry too large, the max entry size is 10, the size of data is 20", + &tikverr.ErrKeyTooLarge{KeySize: math.MaxUint16 + 1}: "key is too large, the size of given key is 65536", + } + for err, errString := range err2str { + tidbErr := ToTiDBErr(err) + assert.NotNil(t, tidbErr) + assert.Contains(t, tidbErr.Error(), errString) + } +} diff --git a/pkg/store/driver/txn/BUILD.bazel b/pkg/store/driver/txn/BUILD.bazel index 4e9cfcade7447..5959cf183a16c 100644 --- a/pkg/store/driver/txn/BUILD.bazel +++ b/pkg/store/driver/txn/BUILD.bazel @@ -54,7 +54,7 @@ go_test( ], embed = [":txn"], flaky = True, - shard_count = 5, + shard_count = 6, deps = [ "//pkg/kv", "//pkg/testkit/testsetup", diff --git a/pkg/store/driver/txn/driver_test.go b/pkg/store/driver/txn/driver_test.go index 815263a6823ce..677fd179f7037 100644 --- a/pkg/store/driver/txn/driver_test.go +++ b/pkg/store/driver/txn/driver_test.go @@ -84,3 +84,7 @@ func TestWriteConflictPrettyFormat(t *testing.T) { "key=‹›‹{metaKey=true, key=DB:56, field=TID:108}, originalKey=6d44423a3536000000fc00000000000000685449443a31303800fe, primary=›‹›‹{metaKey=true, key=DB:56, field=TID:108}, originalPrimaryKey=6d44423a3536000000fc00000000000000685449443a31303800fe›, reason=Optimistic " + kv.TxnRetryableMark require.EqualError(t, newWriteConflictError(conflict), expectedStr) } + +func TestKeyValueOversize(t *testing.T) { + //key := make([]byte, math.MaxUint16+1) +} From 2e79a1ab002117bb7a021f3768f5f0c9e9c7e823 Mon Sep 17 00:00:00 2001 From: you06 Date: Mon, 18 Nov 2024 16:45:46 +0900 Subject: [PATCH 02/13] update client-go and add regression test Signed-off-by: you06 --- DEPS.bzl | 24 ++++++++++++------------ go.mod | 4 ++-- go.sum | 8 ++++---- pkg/session/test/txn/txn_test.go | 26 ++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index b80bfcf36544d..0ad71ad4adb74 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -5776,13 +5776,13 @@ def go_deps(): name = "com_github_pingcap_kvproto", build_file_proto_mode = "disable_global", importpath = "github.com/pingcap/kvproto", - sha256 = "6f2a7d747d05ae61a8f4a3c066058fa69f724480f8dc4a427d66fd066ce730c7", - strip_prefix = "github.com/pingcap/kvproto@v0.0.0-20240924080114-4a3e17f5e62d", + sha256 = "d470ef683433f2c5bc7a1e610da44d516908d326a0341c07208af76a30f0d8a6", + strip_prefix = "github.com/pingcap/kvproto@v0.0.0-20241113043844-e1fa7ea8c302", urls = [ - "http://bazel-cache.pingcap.net:8080/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20240924080114-4a3e17f5e62d.zip", - "http://ats.apps.svc/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20240924080114-4a3e17f5e62d.zip", - "https://cache.hawkingrei.com/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20240924080114-4a3e17f5e62d.zip", - "https://storage.googleapis.com/pingcapmirror/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20240924080114-4a3e17f5e62d.zip", + "http://bazel-cache.pingcap.net:8080/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20241113043844-e1fa7ea8c302.zip", + "http://ats.apps.svc/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20241113043844-e1fa7ea8c302.zip", + "https://cache.hawkingrei.com/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20241113043844-e1fa7ea8c302.zip", + "https://storage.googleapis.com/pingcapmirror/gomod/github.com/pingcap/kvproto/com_github_pingcap_kvproto-v0.0.0-20241113043844-e1fa7ea8c302.zip", ], ) go_repository( @@ -6933,13 +6933,13 @@ def go_deps(): name = "com_github_tikv_client_go_v2", build_file_proto_mode = "disable_global", importpath = "github.com/tikv/client-go/v2", - sha256 = "537e3204b8178e2ce0ce43c744fc2699883bb33e718e267da2f1dd6c389968c2", - strip_prefix = "github.com/tikv/client-go/v2@v2.0.8-0.20241111090227-70049ae310bf", + sha256 = "b0ffe7aa587ea9415209af1bf5e59de94440c687134acedee8a68e65e6ebf289", + strip_prefix = "github.com/tikv/client-go/v2@v2.0.8-0.20241118072114-8b0547ae39ad", urls = [ - "http://bazel-cache.pingcap.net:8080/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241111090227-70049ae310bf.zip", - "http://ats.apps.svc/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241111090227-70049ae310bf.zip", - "https://cache.hawkingrei.com/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241111090227-70049ae310bf.zip", - "https://storage.googleapis.com/pingcapmirror/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241111090227-70049ae310bf.zip", + "http://bazel-cache.pingcap.net:8080/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", + "http://ats.apps.svc/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", + "https://cache.hawkingrei.com/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", + "https://storage.googleapis.com/pingcapmirror/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", ], ) go_repository( diff --git a/go.mod b/go.mod index da24852cfb373..06be8ea7d6352 100644 --- a/go.mod +++ b/go.mod @@ -87,7 +87,7 @@ require ( github.com/pingcap/errors v0.11.5-0.20240318064555-6bd07397691f github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86 github.com/pingcap/fn v1.0.0 - github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d + github.com/pingcap/kvproto v0.0.0-20241113043844-e1fa7ea8c302 github.com/pingcap/log v1.1.1-0.20240314023424-862ccc32f18d github.com/pingcap/sysutil v1.0.1-0.20240311050922-ae81ee01f3a5 github.com/pingcap/tidb/pkg/parser v0.0.0-20211011031125-9b13dc409c5e @@ -110,7 +110,7 @@ require ( github.com/tdakkota/asciicheck v0.2.0 github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2 github.com/tidwall/btree v1.7.0 - github.com/tikv/client-go/v2 v2.0.8-0.20241111090227-70049ae310bf + github.com/tikv/client-go/v2 v2.0.8-0.20241118072114-8b0547ae39ad github.com/tikv/pd/client v0.0.0-20241111073742-238d4d79ea31 github.com/timakin/bodyclose v0.0.0-20240125160201-f835fa56326a github.com/twmb/murmur3 v1.1.6 diff --git a/go.sum b/go.sum index 7322522665ab5..4086c6ccb3529 100644 --- a/go.sum +++ b/go.sum @@ -672,8 +672,8 @@ github.com/pingcap/fn v1.0.0/go.mod h1:u9WZ1ZiOD1RpNhcI42RucFh/lBuzTu6rw88a+oF2Z github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989 h1:surzm05a8C9dN8dIUmo4Be2+pMRb6f55i+UIYrluu2E= github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw= github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w= -github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d h1:vSdKTrF6kpcd56G5BLP0Bz88Nho2tDo7IR1+oSsBAfc= -github.com/pingcap/kvproto v0.0.0-20240924080114-4a3e17f5e62d/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= +github.com/pingcap/kvproto v0.0.0-20241113043844-e1fa7ea8c302 h1:ynwwqr0rLliSOJcx0wHMu4T/NiPXHlK48mk2DCrBKCI= +github.com/pingcap/kvproto v0.0.0-20241113043844-e1fa7ea8c302/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= github.com/pingcap/log v0.0.0-20210625125904-98ed8e2eb1c7/go.mod h1:8AanEdAHATuRurdGxZXBz0At+9avep+ub7U1AGYLIMM= github.com/pingcap/log v1.1.0/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4= github.com/pingcap/log v1.1.1-0.20240314023424-862ccc32f18d h1:y3EueKVfVykdpTyfUnQGqft0ud+xVFuCdp1XkVL0X1E= @@ -826,8 +826,8 @@ github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a h1:J/YdBZ46WKpXsxsW github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a/go.mod h1:h4xBhSNtOeEosLJ4P7JyKXX7Cabg7AVkWCK5gV2vOrM= github.com/tidwall/btree v1.7.0 h1:L1fkJH/AuEh5zBnnBbmTwQ5Lt+bRJ5A8EWecslvo9iI= github.com/tidwall/btree v1.7.0/go.mod h1:twD9XRA5jj9VUQGELzDO4HPQTNJsoWWfYEL+EUQ2cKY= -github.com/tikv/client-go/v2 v2.0.8-0.20241111090227-70049ae310bf h1:qCi6BiBUPk3Ky4f2CCgBxgUmi3ZpuQLYDLgxw1ilXPA= -github.com/tikv/client-go/v2 v2.0.8-0.20241111090227-70049ae310bf/go.mod h1:p9zPFlKBrxhp3b/cBmKBWL9M0X4HtJjgi1ThUtQYF7o= +github.com/tikv/client-go/v2 v2.0.8-0.20241118072114-8b0547ae39ad h1:+JS1f3q7pzMzxpU2fe24kXjY60CexYw8Uy+sYLxGpe8= +github.com/tikv/client-go/v2 v2.0.8-0.20241118072114-8b0547ae39ad/go.mod h1:NI2GfVlB9n7DsIGCxrKcD4psrcuFNEV8m1BgyzK1Amc= github.com/tikv/pd/client v0.0.0-20241111073742-238d4d79ea31 h1:oAYc4m5Eu1OY9ogJ103VO47AYPHvhtzbUPD8L8B67Qk= github.com/tikv/pd/client v0.0.0-20241111073742-238d4d79ea31/go.mod h1:W5a0sDadwUpI9k8p7M77d3jo253ZHdmua+u4Ho4Xw8U= github.com/timakin/bodyclose v0.0.0-20240125160201-f835fa56326a h1:A6uKudFIfAEpoPdaal3aSqGxBzLyU8TqyXImLwo6dIo= diff --git a/pkg/session/test/txn/txn_test.go b/pkg/session/test/txn/txn_test.go index 8d960fcffaf0e..9a40530c76427 100644 --- a/pkg/session/test/txn/txn_test.go +++ b/pkg/session/test/txn/txn_test.go @@ -553,3 +553,29 @@ func TestMemBufferSnapshotRead(t *testing.T) { tk.MustExec("set session tidb_max_chunk_size=default;") tk.MustExec("set session tidb_index_join_batch_size = default") } + +func TestMemBufferCleanupMemoryLeak(t *testing.T) { + // Test if cleanup memory will cause a memory leak. + // When an in-txn statement fails, TiDB cleans up the mutations from this statement. + // If there's a memory leak, the memory usage could increase uncontrollably with retries. + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t(a varchar(255) primary key)") + key1 := strings.Repeat("a", 255) + key2 := strings.Repeat("b", 255) + tk.MustExec(`set global tidb_mem_oom_action='cancel'`) + tk.MustExec("set session tidb_mem_quota_query=10240") + tk.MustExec("begin") + tk.MustExec("insert into t values(?)", key2) + for i := 0; i < 100; i++ { + // The insert statement will fail because of the duplicate key error. + err := tk.ExecToErr("insert into t values(?), (?)", key1, key2) + require.Error(t, err) + if strings.Contains(err.Error(), "Duplicate") { + continue + } + require.NoError(t, err) + } + tk.MustExec("commit") +} From 8b41898998a3103e93b565bb64dd2113210a5b50 Mon Sep 17 00:00:00 2001 From: you06 Date: Mon, 18 Nov 2024 17:03:39 +0900 Subject: [PATCH 03/13] update bazel Signed-off-by: you06 --- pkg/session/test/txn/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/session/test/txn/BUILD.bazel b/pkg/session/test/txn/BUILD.bazel index 351b06e8e07fa..456c700f3123a 100644 --- a/pkg/session/test/txn/BUILD.bazel +++ b/pkg/session/test/txn/BUILD.bazel @@ -9,7 +9,7 @@ go_test( ], flaky = True, race = "on", - shard_count = 9, + shard_count = 10, deps = [ "//pkg/config", "//pkg/kv", From 93f332fb55ae6bb03fc1ed451f7adafa507258e0 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 13:20:18 +0900 Subject: [PATCH 04/13] update client-go Signed-off-by: you06 --- DEPS.bzl | 12 ++++++------ go.mod | 2 +- go.sum | 4 ++-- pkg/kv/key.go | 6 +----- pkg/kv/key_test.go | 2 +- pkg/store/driver/txn/BUILD.bazel | 2 +- pkg/store/driver/txn/driver_test.go | 4 ---- 7 files changed, 12 insertions(+), 20 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 0ad71ad4adb74..e0e72e6ce631c 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -6933,13 +6933,13 @@ def go_deps(): name = "com_github_tikv_client_go_v2", build_file_proto_mode = "disable_global", importpath = "github.com/tikv/client-go/v2", - sha256 = "b0ffe7aa587ea9415209af1bf5e59de94440c687134acedee8a68e65e6ebf289", - strip_prefix = "github.com/tikv/client-go/v2@v2.0.8-0.20241118072114-8b0547ae39ad", + sha256 = "4bc779621156c4ee6f46b57235da9c34c8ec0ee6d3be5f52e33da4c47098eeed", + strip_prefix = "github.com/tikv/client-go/v2@v2.0.8-0.20241120024459-05d115b3e88b", urls = [ - "http://bazel-cache.pingcap.net:8080/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", - "http://ats.apps.svc/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", - "https://cache.hawkingrei.com/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", - "https://storage.googleapis.com/pingcapmirror/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241118072114-8b0547ae39ad.zip", + "http://bazel-cache.pingcap.net:8080/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241120024459-05d115b3e88b.zip", + "http://ats.apps.svc/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241120024459-05d115b3e88b.zip", + "https://cache.hawkingrei.com/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241120024459-05d115b3e88b.zip", + "https://storage.googleapis.com/pingcapmirror/gomod/github.com/tikv/client-go/v2/com_github_tikv_client_go_v2-v2.0.8-0.20241120024459-05d115b3e88b.zip", ], ) go_repository( diff --git a/go.mod b/go.mod index 06be8ea7d6352..d9122f2beb208 100644 --- a/go.mod +++ b/go.mod @@ -110,7 +110,7 @@ require ( github.com/tdakkota/asciicheck v0.2.0 github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2 github.com/tidwall/btree v1.7.0 - github.com/tikv/client-go/v2 v2.0.8-0.20241118072114-8b0547ae39ad + github.com/tikv/client-go/v2 v2.0.8-0.20241120024459-05d115b3e88b github.com/tikv/pd/client v0.0.0-20241111073742-238d4d79ea31 github.com/timakin/bodyclose v0.0.0-20240125160201-f835fa56326a github.com/twmb/murmur3 v1.1.6 diff --git a/go.sum b/go.sum index 4086c6ccb3529..7f4487adaaba1 100644 --- a/go.sum +++ b/go.sum @@ -826,8 +826,8 @@ github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a h1:J/YdBZ46WKpXsxsW github.com/tiancaiamao/gp v0.0.0-20221230034425-4025bc8a4d4a/go.mod h1:h4xBhSNtOeEosLJ4P7JyKXX7Cabg7AVkWCK5gV2vOrM= github.com/tidwall/btree v1.7.0 h1:L1fkJH/AuEh5zBnnBbmTwQ5Lt+bRJ5A8EWecslvo9iI= github.com/tidwall/btree v1.7.0/go.mod h1:twD9XRA5jj9VUQGELzDO4HPQTNJsoWWfYEL+EUQ2cKY= -github.com/tikv/client-go/v2 v2.0.8-0.20241118072114-8b0547ae39ad h1:+JS1f3q7pzMzxpU2fe24kXjY60CexYw8Uy+sYLxGpe8= -github.com/tikv/client-go/v2 v2.0.8-0.20241118072114-8b0547ae39ad/go.mod h1:NI2GfVlB9n7DsIGCxrKcD4psrcuFNEV8m1BgyzK1Amc= +github.com/tikv/client-go/v2 v2.0.8-0.20241120024459-05d115b3e88b h1:/hmt2FCt34rCVBX9dswiSdHOkppP67VWaESryTxDKc8= +github.com/tikv/client-go/v2 v2.0.8-0.20241120024459-05d115b3e88b/go.mod h1:NI2GfVlB9n7DsIGCxrKcD4psrcuFNEV8m1BgyzK1Amc= github.com/tikv/pd/client v0.0.0-20241111073742-238d4d79ea31 h1:oAYc4m5Eu1OY9ogJ103VO47AYPHvhtzbUPD8L8B67Qk= github.com/tikv/pd/client v0.0.0-20241111073742-238d4d79ea31/go.mod h1:W5a0sDadwUpI9k8p7M77d3jo253ZHdmua+u4Ho4Xw8U= github.com/timakin/bodyclose v0.0.0-20240125160201-f835fa56326a h1:A6uKudFIfAEpoPdaal3aSqGxBzLyU8TqyXImLwo6dIo= diff --git a/pkg/kv/key.go b/pkg/kv/key.go index b2af3d3e78b2f..066e35ea09adf 100644 --- a/pkg/kv/key.go +++ b/pkg/kv/key.go @@ -98,10 +98,6 @@ func (k Key) String() string { type KeyRange struct { StartKey Key EndKey Key - - XXXNoUnkeyedLiteral struct{} - XXXunrecognized []byte - XXXsizecache int32 } // KeyRangeSliceMemUsage return the memory usage of []KeyRange @@ -110,7 +106,7 @@ func KeyRangeSliceMemUsage(k []KeyRange) int64 { res := sizeofKeyRange * int64(cap(k)) for _, m := range k { - res += int64(cap(m.StartKey)) + int64(cap(m.EndKey)) + int64(cap(m.XXXunrecognized)) + res += int64(cap(m.StartKey)) + int64(cap(m.EndKey)) } return res diff --git a/pkg/kv/key_test.go b/pkg/kv/key_test.go index 0afacd882c829..9f1afe27b9199 100644 --- a/pkg/kv/key_test.go +++ b/pkg/kv/key_test.go @@ -345,7 +345,7 @@ func TestKeyRangeDefinition(t *testing.T) { StartKey: []byte("s2"), EndKey: []byte("e2"), }} - require.Equal(t, int64(168), KeyRangeSliceMemUsage(s)) + require.Equal(t, int64(104), KeyRangeSliceMemUsage(s)) } func BenchmarkIsPoint(b *testing.B) { diff --git a/pkg/store/driver/txn/BUILD.bazel b/pkg/store/driver/txn/BUILD.bazel index 5959cf183a16c..4e9cfcade7447 100644 --- a/pkg/store/driver/txn/BUILD.bazel +++ b/pkg/store/driver/txn/BUILD.bazel @@ -54,7 +54,7 @@ go_test( ], embed = [":txn"], flaky = True, - shard_count = 6, + shard_count = 5, deps = [ "//pkg/kv", "//pkg/testkit/testsetup", diff --git a/pkg/store/driver/txn/driver_test.go b/pkg/store/driver/txn/driver_test.go index 677fd179f7037..815263a6823ce 100644 --- a/pkg/store/driver/txn/driver_test.go +++ b/pkg/store/driver/txn/driver_test.go @@ -84,7 +84,3 @@ func TestWriteConflictPrettyFormat(t *testing.T) { "key=‹›‹{metaKey=true, key=DB:56, field=TID:108}, originalKey=6d44423a3536000000fc00000000000000685449443a31303800fe, primary=›‹›‹{metaKey=true, key=DB:56, field=TID:108}, originalPrimaryKey=6d44423a3536000000fc00000000000000685449443a31303800fe›, reason=Optimistic " + kv.TxnRetryableMark require.EqualError(t, newWriteConflictError(conflict), expectedStr) } - -func TestKeyValueOversize(t *testing.T) { - //key := make([]byte, math.MaxUint16+1) -} From af8b4f3265a2afc3a3deb27a263b8e4b055580e9 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 13:43:11 +0900 Subject: [PATCH 05/13] explicitly close snapshot iterator Signed-off-by: you06 --- pkg/executor/adapter.go | 2 +- pkg/executor/mem_reader.go | 22 +++++++++++++++++++--- pkg/executor/union_scan.go | 3 +++ pkg/executor/union_scan_test.go | 15 +++++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/pkg/executor/adapter.go b/pkg/executor/adapter.go index 431efd09da0c6..568bca1a308aa 100644 --- a/pkg/executor/adapter.go +++ b/pkg/executor/adapter.go @@ -659,7 +659,7 @@ func (a *ExecStmt) handleStmtForeignKeyTrigger(ctx context.Context, e exec.Execu if stmtCtx.ForeignKeyTriggerCtx.HasFKCascades { // If the ExecStmt has foreign key cascade to be executed, we need call `StmtCommit` to commit the ExecStmt itself // change first. - // Since `UnionScanExec` use `SnapshotIter` and `SnapshotGetter` to read txn mem-buffer, if we don't do `StmtCommit`, + // Since `UnionScanExec` use `SnapshotIter` and `SnapshotGetter` to read txn mem-buffer, if we don't do `StmtCommit`, // then the fk cascade executor can't read the mem-buffer changed by the ExecStmt. a.Ctx.StmtCommit(ctx) } diff --git a/pkg/executor/mem_reader.go b/pkg/executor/mem_reader.go index a86da267c92af..596f940514cb2 100644 --- a/pkg/executor/mem_reader.go +++ b/pkg/executor/mem_reader.go @@ -345,7 +345,6 @@ func (iter *txnMemBufferIter) Valid() bool { if iter.curr.Valid() { return true } - iter.curr = nil iter.idx++ } for iter.idx < len(iter.kvRanges) { @@ -372,7 +371,6 @@ func (iter *txnMemBufferIter) Valid() bool { if iter.curr.Valid() { return true } - iter.curr = nil iter.idx++ } return false @@ -398,7 +396,10 @@ func (iter *txnMemBufferIter) Value() []byte { return iter.curr.Value() } -func (*txnMemBufferIter) Close() { +func (iter *txnMemBufferIter) Close() { + if iter.curr != nil { + iter.curr.Close() + } } func (m *memTableReader) getMemRowsIter(ctx context.Context) (memRowsIter, error) { @@ -882,6 +883,7 @@ func buildMemIndexMergeReader(ctx context.Context, us *UnionScanExec, indexMerge type memRowsIter interface { Next() ([]types.Datum, error) + Close() } type defaultRowsIter struct { @@ -898,6 +900,8 @@ func (iter *defaultRowsIter) Next() ([]types.Datum, error) { return nil, nil } +func (iter *defaultRowsIter) Close() {} + // memRowsIterForTable combine a kv.Iterator and a kv decoder to get a memRowsIter. type memRowsIterForTable struct { kvIter *txnMemBufferIter // txnMemBufferIter is the kv.Iterator @@ -966,6 +970,12 @@ func (iter *memRowsIterForTable) Next() ([]types.Datum, error) { return ret, nil } +func (iter *memRowsIterForTable) Close() { + if iter.kvIter != nil { + iter.kvIter.Close() + } +} + type memRowsIterForIndex struct { kvIter *txnMemBufferIter tps []*types.FieldType @@ -1018,6 +1028,12 @@ func (iter *memRowsIterForIndex) Next() ([]types.Datum, error) { return ret, nil } +func (iter *memRowsIterForIndex) Close() { + if iter.kvIter != nil { + iter.kvIter.Close() + } +} + func (m *memIndexMergeReader) getMemRowsIter(ctx context.Context) (memRowsIter, error) { data, err := m.getMemRows(ctx) if err != nil { diff --git a/pkg/executor/union_scan.go b/pkg/executor/union_scan.go index 5799ad478093f..5f6d0cd8c26d6 100644 --- a/pkg/executor/union_scan.go +++ b/pkg/executor/union_scan.go @@ -191,6 +191,9 @@ func (us *UnionScanExec) Close() error { us.cursor4AddRows = nil us.cursor4SnapshotRows = 0 us.snapshotRows = us.snapshotRows[:0] + if us.addedRowsIter != nil { + us.addedRowsIter.Close() + } return exec.Close(us.Children(0)) } diff --git a/pkg/executor/union_scan_test.go b/pkg/executor/union_scan_test.go index f42696db283e6..2a1f982ebf76c 100644 --- a/pkg/executor/union_scan_test.go +++ b/pkg/executor/union_scan_test.go @@ -360,6 +360,21 @@ func TestIssue32422(t *testing.T) { tk.MustExec("rollback") } +func TestSnapshotWithConcurrentWrite(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t1 (id int auto_increment key, b int, index(b));") + + tk.MustExec("begin") + tk.MustExec("insert into t1 (b) values (1),(2),(3),(4),(5),(6),(7),(8);") + for j := 0; j < 16; j++ { + tk.MustExec("insert into t1 (b) select /*+ use_index(t1, b) */ id from t1;") + } + tk.MustQuery("select count(1) from t1").Check(testkit.Rows("524288")) // 8 * 2^16 rows + tk.MustExec("rollback") +} + func BenchmarkUnionScanRead(b *testing.B) { store := testkit.CreateMockStore(b) From 85406bd484c6657f3f9bf0f7a86a823de09b97ff Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 13:57:51 +0900 Subject: [PATCH 06/13] fix lint Signed-off-by: you06 --- pkg/executor/mem_reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/executor/mem_reader.go b/pkg/executor/mem_reader.go index 596f940514cb2..a6ea7be89b2a3 100644 --- a/pkg/executor/mem_reader.go +++ b/pkg/executor/mem_reader.go @@ -900,7 +900,7 @@ func (iter *defaultRowsIter) Next() ([]types.Datum, error) { return nil, nil } -func (iter *defaultRowsIter) Close() {} +func (*defaultRowsIter) Close() {} // memRowsIterForTable combine a kv.Iterator and a kv decoder to get a memRowsIter. type memRowsIterForTable struct { From 0e7ebb3e5c76daa1c0455b1ff69fed5bfac0db84 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 14:21:58 +0900 Subject: [PATCH 07/13] comment Close func of memRowsIter Signed-off-by: you06 --- pkg/executor/mem_reader.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/executor/mem_reader.go b/pkg/executor/mem_reader.go index a6ea7be89b2a3..7fb687672ee57 100644 --- a/pkg/executor/mem_reader.go +++ b/pkg/executor/mem_reader.go @@ -883,6 +883,7 @@ func buildMemIndexMergeReader(ctx context.Context, us *UnionScanExec, indexMerge type memRowsIter interface { Next() ([]types.Datum, error) + // Close will release the snapshot it holds, so be sure to call Close. Close() } From 150bdd88d540fa62e30a56e070d4c84f7f1748aa Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 15:00:13 +0900 Subject: [PATCH 08/13] fix test Signed-off-by: you06 --- pkg/executor/executor_pkg_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/executor/executor_pkg_test.go b/pkg/executor/executor_pkg_test.go index b965d240b1732..46666505edf80 100644 --- a/pkg/executor/executor_pkg_test.go +++ b/pkg/executor/executor_pkg_test.go @@ -131,7 +131,7 @@ func TestBuildKvRangesForIndexJoinWithoutCwcAndWithMemoryTracker(t *testing.T) { } require.Equal(t, 2*bytesConsumed1, bytesConsumed2) - require.Equal(t, int64(25560), bytesConsumed1) + require.Equal(t, int64(23640), bytesConsumed1) } func generateIndexRange(vals ...int64) *ranger.Range { From 40f7b9f8193b610219a7f860dc626c7d8d8e323e Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 16:15:29 +0900 Subject: [PATCH 09/13] fix flaky test Signed-off-by: you06 --- tests/integrationtest/r/globalindex/insert.result | 2 +- tests/integrationtest/t/globalindex/insert.test | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrationtest/r/globalindex/insert.result b/tests/integrationtest/r/globalindex/insert.result index 74e53a66c2f7e..e873815ea20ff 100644 --- a/tests/integrationtest/r/globalindex/insert.result +++ b/tests/integrationtest/r/globalindex/insert.result @@ -6,7 +6,7 @@ a b 1 3 alter table t add unique index idx1(b) global; insert into t values (2, 4), (3, 4) on duplicate key update a=2, b=5; -select * from t use index (idx1); +select * from t use index (idx1) order by a desc; a b 2 5 1 3 diff --git a/tests/integrationtest/t/globalindex/insert.test b/tests/integrationtest/t/globalindex/insert.test index e4251d662a822..9e641354a341c 100644 --- a/tests/integrationtest/t/globalindex/insert.test +++ b/tests/integrationtest/t/globalindex/insert.test @@ -5,6 +5,6 @@ select * from t use index (idx); alter table t add unique index idx1(b) global; insert into t values (2, 4), (3, 4) on duplicate key update a=2, b=5; -select * from t use index (idx1); +select * from t use index (idx1) order by a desc; From d08c5ff0a4dd7a83ce1e2e8366b28f13bd6f281a Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 18:09:53 +0900 Subject: [PATCH 10/13] fix test Signed-off-by: you06 --- tests/integrationtest/r/executor/ddl.result | 2 +- tests/integrationtest/t/executor/ddl.test | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrationtest/r/executor/ddl.result b/tests/integrationtest/r/executor/ddl.result index b7c75847b74b1..08ddc4cf63b84 100644 --- a/tests/integrationtest/r/executor/ddl.result +++ b/tests/integrationtest/r/executor/ddl.result @@ -379,7 +379,7 @@ create or replace definer='root'@'localhost' view v_nested as select * from v_ne Error 1146 (42S02): Table 'executor__ddl.v_nested' doesn't exist drop table test_v_nested; drop view v_nested, v_nested2; -create view v_stale as select * from source_table as of timestamp current_timestamp(3); +create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 minute); Error 1356 (HY000): View 'executor__ddl.v_stale' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them drop view if exists v1,v2; drop table if exists t1; diff --git a/tests/integrationtest/t/executor/ddl.test b/tests/integrationtest/t/executor/ddl.test index 59bdbcc68cfa1..7f1ac8af6da40 100644 --- a/tests/integrationtest/t/executor/ddl.test +++ b/tests/integrationtest/t/executor/ddl.test @@ -314,7 +314,7 @@ drop table test_v_nested; drop view v_nested, v_nested2; ## Refer https://github.com/pingcap/tidb/issues/25876 -- error 1356 -create view v_stale as select * from source_table as of timestamp current_timestamp(3); +create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 minute); ## Refer https://github.com/pingcap/tidb/issues/32682 drop view if exists v1,v2; drop table if exists t1; From 9541ba0c001b1b65f6f1d5537bbcb9e909a10d43 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 18:34:58 +0900 Subject: [PATCH 11/13] guarantee the read ts can see schema Signed-off-by: you06 --- tests/integrationtest/r/executor/ddl.result | 3 ++- tests/integrationtest/t/executor/ddl.test | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integrationtest/r/executor/ddl.result b/tests/integrationtest/r/executor/ddl.result index 08ddc4cf63b84..6d4e00ccde74b 100644 --- a/tests/integrationtest/r/executor/ddl.result +++ b/tests/integrationtest/r/executor/ddl.result @@ -379,7 +379,8 @@ create or replace definer='root'@'localhost' view v_nested as select * from v_ne Error 1146 (42S02): Table 'executor__ddl.v_nested' doesn't exist drop table test_v_nested; drop view v_nested, v_nested2; -create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 minute); +select sleep(1); +create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 second); Error 1356 (HY000): View 'executor__ddl.v_stale' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them drop view if exists v1,v2; drop table if exists t1; diff --git a/tests/integrationtest/t/executor/ddl.test b/tests/integrationtest/t/executor/ddl.test index 7f1ac8af6da40..f77540a8eda95 100644 --- a/tests/integrationtest/t/executor/ddl.test +++ b/tests/integrationtest/t/executor/ddl.test @@ -314,7 +314,8 @@ drop table test_v_nested; drop view v_nested, v_nested2; ## Refer https://github.com/pingcap/tidb/issues/25876 -- error 1356 -create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 minute); +select sleep(1); +create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 second); ## Refer https://github.com/pingcap/tidb/issues/32682 drop view if exists v1,v2; drop table if exists t1; From a22f433d1382d85e465cdf5f4a6a58a38772df1f Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 19:10:05 +0900 Subject: [PATCH 12/13] fix test Signed-off-by: you06 --- tests/integrationtest/t/executor/ddl.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrationtest/t/executor/ddl.test b/tests/integrationtest/t/executor/ddl.test index f77540a8eda95..def024b766479 100644 --- a/tests/integrationtest/t/executor/ddl.test +++ b/tests/integrationtest/t/executor/ddl.test @@ -313,8 +313,8 @@ create or replace definer='root'@'localhost' view v_nested as select * from v_ne drop table test_v_nested; drop view v_nested, v_nested2; ## Refer https://github.com/pingcap/tidb/issues/25876 --- error 1356 select sleep(1); +-- error 1356 create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 second); ## Refer https://github.com/pingcap/tidb/issues/32682 drop view if exists v1,v2; From b58330ae516b5d627d8325803b7e148200cf2672 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 20 Nov 2024 19:37:33 +0900 Subject: [PATCH 13/13] fix test Signed-off-by: you06 --- tests/integrationtest/r/executor/ddl.result | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integrationtest/r/executor/ddl.result b/tests/integrationtest/r/executor/ddl.result index 6d4e00ccde74b..cbf2c26cb29b4 100644 --- a/tests/integrationtest/r/executor/ddl.result +++ b/tests/integrationtest/r/executor/ddl.result @@ -380,6 +380,8 @@ Error 1146 (42S02): Table 'executor__ddl.v_nested' doesn't exist drop table test_v_nested; drop view v_nested, v_nested2; select sleep(1); +sleep(1) +0 create view v_stale as select * from source_table as of timestamp date_sub(current_timestamp(3), interval 1 second); Error 1356 (HY000): View 'executor__ddl.v_stale' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them drop view if exists v1,v2;