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.

Sign up for GitHub

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

Jump to bottom

variable: support tidb_schema_cache_size reasonable range #54035

Merged
merged 10 commits into from
Jun 26, 2024
Merged

variable: support tidb_schema_cache_size reasonable range #54035

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0d00c41
variable: fix tidb_schema_cache_size range max value
lilinghai Jun 14, 2024
8b45f6b
variable: support tidb_schema_cache_size min/max value and byte format
lilinghai Jun 14, 2024
8487d34
fix ci
lilinghai Jun 15, 2024
10fe6b4
fix ut
lilinghai Jun 15, 2024
107d196
fix conflict
lilinghai Jun 24, 2024
b0fb3e4
more test
lilinghai Jun 24, 2024
be15c02
use std parse bytes pkg
lilinghai Jun 25, 2024
6fa1066
fix ci
lilinghai Jun 25, 2024
394f339
use standard mib unit
lilinghai Jun 25, 2024
646b75e
revert to parseByteSize
lilinghai Jun 26, 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
2 changes: 1 addition & 1 deletion pkg/infoschema/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ func NewBuilder(r autoid.Requirement, factory func() (pools.Resource, error), in
}
schemaCacheSize := variable.SchemaCacheSize.Load()
if schemaCacheSize > 0 {
infoData.tableCache.SetCapacity(uint64(schemaCacheSize))
infoData.tableCache.SetCapacity(schemaCacheSize)
builder.enableV2 = true
}
return builder
Expand Down
12 changes: 6 additions & 6 deletions pkg/infoschema/infoschema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,13 +953,13 @@ func TestEnableInfoSchemaV2(t *testing.T) {
// Test the @@tidb_enable_infoschema_v2 variable.
tk.MustQuery("select @@tidb_schema_cache_size").Check(testkit.Rows("0"))
tk.MustQuery("select @@global.tidb_schema_cache_size").Check(testkit.Rows("0"))
require.Equal(t, variable.SchemaCacheSize.Load(), int64(0))
require.Equal(t, variable.SchemaCacheSize.Load(), uint64(0))

// Modify it.
tk.MustExec("set @@global.tidb_schema_cache_size = 1024")
tk.MustQuery("select @@global.tidb_schema_cache_size").Check(testkit.Rows("1024"))
tk.MustQuery("select @@tidb_schema_cache_size").Check(testkit.Rows("1024"))
require.Equal(t, variable.SchemaCacheSize.Load(), int64(1024))
tk.MustExec("set @@global.tidb_schema_cache_size = 1073741824")
lance6716 marked this conversation as resolved.
Show resolved Hide resolved
tk.MustQuery("select @@global.tidb_schema_cache_size").Check(testkit.Rows("1073741824"))
tk.MustQuery("select @@tidb_schema_cache_size").Check(testkit.Rows("1073741824"))
require.Equal(t, variable.SchemaCacheSize.Load(), uint64(1073741824))

tk.MustExec("use test")
tk.MustExec("create table v2 (id int)")
Expand All @@ -982,7 +982,7 @@ func TestEnableInfoSchemaV2(t *testing.T) {
// Change infoschema back to v1 and check again.
tk.MustExec("set @@global.tidb_schema_cache_size = 0")
tk.MustQuery("select @@global.tidb_schema_cache_size").Check(testkit.Rows("0"))
require.Equal(t, variable.SchemaCacheSize.Load(), int64(0))
require.Equal(t, variable.SchemaCacheSize.Load(), uint64(0))

tk.MustExec("drop table v1")
is = domain.GetDomain(tk.Session()).InfoSchema()
Expand Down
14 changes: 7 additions & 7 deletions pkg/infoschema/test/infoschemav2test/v2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func TestSpecialSchemas(t *testing.T) {
require.NoError(t, tk.Session().Auth(&auth.UserIdentity{Username: "root", Hostname: "%"}, nil, nil, nil))
tk.MustExec("use test")

tk.MustExec("set @@global.tidb_schema_cache_size = 1024;")
tk.MustQuery("select @@global.tidb_schema_cache_size;").Check(testkit.Rows("1024"))
tk.MustExec("set @@global.tidb_schema_cache_size = 1073741824;")
tk.MustQuery("select @@global.tidb_schema_cache_size;").Check(testkit.Rows("1073741824"))
tk.MustExec("create table t (id int);")
is := domain.GetDomain(tk.Session()).InfoSchema()
isV2, _ := infoschema.IsV2(is)
Expand Down Expand Up @@ -247,19 +247,19 @@ func TestTiDBSchemaCacheSizeVariable(t *testing.T) {
ok, raw := infoschema.IsV2(is)
if ok {
val := variable.SchemaCacheSize.Load()
tk.MustQuery("select @@global.tidb_schema_cache_size").CheckContain(strconv.FormatInt(val, 10))
tk.MustQuery("select @@global.tidb_schema_cache_size").CheckContain(strconv.FormatUint(val, 10))

// On start, the capacity might not be set correctly because infoschema have not load global variable yet.
// cap := raw.Data.CacheCapacity()
// require.Equal(t, cap, uint64(val))
}

tk.MustExec("set @@global.tidb_schema_cache_size = 32 * 1024 * 1024")
tk.MustQuery("select @@global.tidb_schema_cache_size").CheckContain("33554432")
require.Equal(t, variable.SchemaCacheSize.Load(), int64(33554432))
tk.MustExec("set @@global.tidb_schema_cache_size = 1024 * 1024 * 1024")
tk.MustQuery("select @@global.tidb_schema_cache_size").CheckContain("1073741824")
require.Equal(t, variable.SchemaCacheSize.Load(), uint64(1073741824))
tk.MustExec("create table trigger_reload (id int)") // need to trigger infoschema rebuild to reset capacity
is = dom.InfoSchema()
ok, raw = infoschema.IsV2(is)
require.True(t, ok)
require.Equal(t, raw.Data.CacheCapacity(), uint64(33554432))
require.Equal(t, raw.Data.CacheCapacity(), uint64(1073741824))
}
29 changes: 21 additions & 8 deletions pkg/sessionctx/variable/sysvar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3114,14 +3114,27 @@ var defaultSysVars = []*SysVar{
}, GetGlobal: func(ctx context.Context, vars *SessionVars) (string, error) {
return BoolToOnOff(EnableCheckConstraint.Load()), nil
}},
{Scope: ScopeGlobal, Name: TiDBSchemaCacheSize, Value: strconv.Itoa(DefTiDBSchemaCacheSize), Type: TypeInt, MinValue: 0, MaxValue: math.MaxInt32, SetGlobal: func(ctx context.Context, vars *SessionVars, val string) error {
// It does not take effect immediately, but within a ddl lease, infoschema reload would cause the v2 to be used.
SchemaCacheSize.Store(TidbOptInt64(val, DefTiDBSchemaCacheSize))
return nil
}, GetGlobal: func(ctx context.Context, vars *SessionVars) (string, error) {
val := SchemaCacheSize.Load()
return strconv.FormatInt(val, 10), nil
}},
{Scope: ScopeGlobal, Name: TiDBSchemaCacheSize, Value: strconv.Itoa(DefTiDBSchemaCacheSize), Type: TypeStr,
Validation: func(s *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) {
_, str, err := parseSchemaCacheSize(s, normalizedValue, originalValue)
if err != nil {
return "", err
}
return str, nil
},
SetGlobal: func(ctx context.Context, vars *SessionVars, val string) error {
// It does not take effect immediately, but within a ddl lease, infoschema reload would cause the v2 to be used.
bt, str, err := parseSchemaCacheSize(vars, val, val)
if err != nil {
return err
}
SchemaCacheSize.Store(bt)
SchemaCacheSizeOriginText.Store(str)
return nil
},
GetGlobal: func(ctx context.Context, vars *SessionVars) (string, error) {
return SchemaCacheSizeOriginText.Load(), nil
}},
{Scope: ScopeSession, Name: TiDBSessionAlias, Value: "", Type: TypeStr,
Validation: func(s *SessionVars, normalizedValue string, originalValue string, _ ScopeFlag) (string, error) {
chars := []rune(normalizedValue)
Expand Down
96 changes: 96 additions & 0 deletions pkg/sessionctx/variable/sysvar_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1671,3 +1671,99 @@ func TestSetEnableColumnTrackingAndPersistAnalyzeOptions(t *testing.T) {
require.NoError(t, err)
require.Equal(t, On, val)
}

func TestTiDBSchemaCacheSize(t *testing.T) {
vars := NewSessionVars(nil)
mock := NewMockGlobalAccessor4Tests()
mock.SessionVars = vars
vars.GlobalVarsAccessor = mock
var (
mb uint64 = 1 << 20
err error
val string
maxValue uint64 = math.MaxInt64
)
// Test tidb_schema_cache_size
schemaCacheSize := GetSysVar(TiDBSchemaCacheSize)
// Check default value
require.Equal(t, schemaCacheSize.Value, strconv.Itoa(DefTiDBSchemaCacheSize))

// MinValue is 512 MB
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, strconv.FormatUint(100*mb, 10))
Copy link
Member

Choose a reason for hiding this comment

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

Please test 512MB-1 and MaxValue + 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, "512MB", val)

// MaxValue is 9223372036854775807
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, strconv.FormatUint(maxValue+1, 10))
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, strconv.FormatUint(maxValue, 10), val)
lance6716 marked this conversation as resolved.
Show resolved Hide resolved

// Test Normal Value
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, strconv.FormatUint(1024*mb, 10))
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, strconv.FormatUint(1024*mb, 10), val)

// Test Close
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, strconv.FormatUint(0, 10))
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, "0", val)

// Test byteSize format
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "1234567890123")
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, SchemaCacheSize.Load(), uint64(1234567890123))
require.Equal(t, "1234567890123", val)

err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "10KB")
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, SchemaCacheSize.Load(), uint64(512<<20))
require.Equal(t, "512MB", val)

err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "12345678KB")
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, SchemaCacheSize.Load(), uint64(12345678<<10))
require.Equal(t, "12345678KB", val)

err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "700MB")
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, SchemaCacheSize.Load(), uint64(700<<20))
require.Equal(t, "700MB", val)

err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "20GB")
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, SchemaCacheSize.Load(), uint64(20<<30))
require.Equal(t, "20GB", val)

err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "2TB")
require.NoError(t, err)
val, err = mock.GetGlobalSysVar(TiDBSchemaCacheSize)
require.NoError(t, err)
require.Equal(t, SchemaCacheSize.Load(), uint64(2<<40))
require.Equal(t, "2TB", val)

// Test error
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "123aaa123")
require.Error(t, err)
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "700MBaa")
require.Error(t, err)
err = mock.SetGlobalSysVar(context.Background(), TiDBSchemaCacheSize, "a700MB")
require.Error(t, err)
}
4 changes: 3 additions & 1 deletion pkg/sessionctx/variable/tidb_vars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"math"
"strconv"
"time"

"github.com/pingcap/tidb/pkg/config"
Expand Down Expand Up @@ -1595,7 +1596,8 @@ var (
IgnoreInlistPlanDigest = atomic.NewBool(DefTiDBIgnoreInlistPlanDigest)
TxnEntrySizeLimit = atomic.NewUint64(DefTiDBTxnEntrySizeLimit)

SchemaCacheSize = atomic.NewInt64(DefTiDBSchemaCacheSize)
SchemaCacheSize = atomic.NewUint64(DefTiDBSchemaCacheSize)
SchemaCacheSizeOriginText = atomic.NewString(strconv.Itoa(DefTiDBSchemaCacheSize))
Copy link
Member

Choose a reason for hiding this comment

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

We can get the text from SchemaCacheSize like we have done for TiDBServerMemoryLimitSessMinSize

Copy link
Contributor Author

@lilinghai lilinghai Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This value is used to store the content set by the user. For example, if the user sets 1GB, then the display for user queries will also be 1GB, not 1,073,741,824. However, the TiDBServerMemoryLimitSessMinSize will display the value as an integer to the user.

mysql> set @@global.tidb_server_memory_limit_sess_min_size='1GB';
Query OK, 0 rows affected (0.01 sec)

mysql> select @@global.tidb_server_memory_limit_sess_min_size;
+-------------------------------------------------+
| @@global.tidb_server_memory_limit_sess_min_size |
+-------------------------------------------------+
| 1073741824                                      |
+-------------------------------------------------+
1 row in set (0.00 sec)

mysql> set @@global.tidb_schema_cache_size='1GB';
Query OK, 0 rows affected (0.00 sec)

mysql> select @@global.tidb_schema_cache_size;
+---------------------------------+
| @@global.tidb_schema_cache_size |
+---------------------------------+
| 1GB                             |
+---------------------------------+
1 row in set (0.00 sec)

simlar to tidb_server_memory_limit

mysql> set @@global.tidb_server_memory_limit='1GB';
Query OK, 0 rows affected (0.06 sec)

mysql> select @@global.tidb_server_memory_limit;
+-----------------------------------+
| @@global.tidb_server_memory_limit |
+-----------------------------------+
| 1GB                               |
+-----------------------------------+
1 row in set (0.00 sec)

)

var (
Expand Down
23 changes: 23 additions & 0 deletions pkg/sessionctx/variable/varsutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"io"
"math"
"slices"
"strconv"
"strings"
Expand Down Expand Up @@ -587,3 +588,25 @@ func ParseAnalyzeSkipColumnTypes(val string) map[string]struct{} {
}
return skipTypes
}

func parseSchemaCacheSize(s *SessionVars, normalizedValue string, originalValue string) (byteSize uint64, normalizedStr string, err error) {
defer func() {
if err == nil && byteSize > 0 && byteSize < (512<<20) {
s.StmtCtx.AppendWarning(ErrTruncatedWrongValue.FastGenByArgs(TiDBSchemaCacheSize, originalValue))
byteSize = 512 << 20
normalizedStr = "512MB"
lance6716 marked this conversation as resolved.
Show resolved Hide resolved
}
if err == nil && byteSize > math.MaxInt64 {
s.StmtCtx.AppendWarning(ErrTruncatedWrongValue.FastGenByArgs(TiDBSchemaCacheSize, originalValue))
byteSize = math.MaxInt64
normalizedStr = strconv.Itoa(math.MaxInt64)
lance6716 marked this conversation as resolved.
Show resolved Hide resolved
}
}()

bt, str := parseByteSize(normalizedValue)
lance6716 marked this conversation as resolved.
Show resolved Hide resolved
if str != "" {
return bt, str, nil
}

return 0, "", ErrTruncatedWrongValue.GenWithStackByArgs(TiDBSchemaCacheSize, originalValue)
}