*: add global system variable `tmp_table_size` #24827

tiancaiamao · 2021-05-21T08:52:10Z

What problem does this PR solve?

Problem Summary:

Use system variable tmp_table_size is used to control the temporary table size threshold.

What is changed and how it works?

What's Changed:

add global + session system variable tmp_table_size
check the temporary table do not exceed this threshold

How it Works:

We can't use the txn.Size() directly, because the data may not located in the temporary table.
So I use the txn size before & after the AddRecord() operation to calculate the temporary table size.

Check List

Tests

Unit test

Need to Update document too

Release note

No release note

tiancaiamao · 2021-05-21T08:58:40Z

/rebuild

djshow832 · 2021-05-24T14:47:45Z

sessionctx/variable/sysvar.go

@@ -1501,6 +1501,12 @@ var defaultSysVars = []*SysVar{
 	{Scope: ScopeGlobal, Name: TiDBGCLifetime, Value: "10m0s", Type: TypeDuration, MinValue: int64(time.Minute * 10), MaxValue: math.MaxInt64},
 	{Scope: ScopeGlobal, Name: TiDBGCConcurrency, Value: "-1", Type: TypeInt, MinValue: 1, MaxValue: 128, AllowAutoValue: true},
 	{Scope: ScopeGlobal, Name: TiDBGCScanLockMode, Value: "PHYSICAL", Type: TypeEnum, PossibleValues: []string{"PHYSICAL", "LEGACY"}},
+
+	// See https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_temptable_max_ram
+	{Scope: ScopeGlobal, Name: TempTableMaxRAM, Value: strconv.Itoa(DefTempTableMaxRAM), Type: TypeInt, MinValue: 2097152, MaxValue: math.MaxInt64, SetGlobal: func(s *SessionVars, val string) error {


For MySQL, the max value is MaxUint64.

It's okay to not follow strictly, but there is an issue in that the internal value is a signed int32, but the sysvar allows up to signed int64. The two types should align.

Signed int32 only allows up to 2GB temporary tables, which is "okay", but there might be some cases where it needs to be larger.

It is still set to have a maximum value of math.MaxInt64, but the internal value is an int32.

I think it's better to be conservative.
We can update the value to be larger anytime as long as we'd like to, but if we set it's initial value huge and turn down the value in a future release, there will be a compatibility issue.
And now I take @morgo 's advise and set it to math.MaxInt32

djshow832 · 2021-05-25T08:54:26Z

Please resolve conflicts, rest LGTM. @tiancaiamao

djshow832 · 2021-05-25T10:21:31Z

/lgtm

table/tables/tables.go

sessionctx/variable/sysvar.go

Co-authored-by: Morgan Tocker <[email protected]>

tisonkun · 2021-06-01T13:51:17Z

Please refer the pull request to its issue #24169 and it would be better if we create a dedicated sub-issue for this pr.

djshow832 · 2021-06-02T02:54:15Z

@tiancaiamao Please create a sub-issue and resolve conflicts, and then ask @morgo and @wjhuang2016 to review it again.

tiancaiamao · 2021-06-03T02:23:48Z

Since in MySQL8 temptable_max_ram is deprecated, we introduce another TiDB specified variable tidb_temptable_max_ram

PTAL @djshow832 @wjhuang2016 @morgo

wjhuang2016

restLGTM

table/tables/tables.go

morgo · 2021-06-03T03:46:15Z

@tiancaiamao As discussed. There are two settings which apply in MySQL 5.7 here:

They are used together, so the smaller limit is what applies to in-memory temporary tables. These limits are enforced per-temporary table, which is the same semantic as what is used in this PR. The buffer for temptable_max_ram in MySQL 8.0 is a global buffer pool shared by all connections (as an administrator I prefer this behavior, as it's easier to plan provisioning of space for temporary queries). I think it's best to avoid this naming convention to prevent confusion.

Where tmp_table_size and max_heap_table_size differ from this implementation is that TiDB currently has no spill to disk mechanism. I actually think that is okay. If you create a temporary table in MySQL and you explicitly say the engine=MEMORY, it will also not spill to disk when the threshold is reached. You will instead get a table-is-full error.

The bigger difference is that tmp_table_size and max_heap_table_size apply to both implicit and explicit temporary tables, whereas in TiDB this setting will only apply to explicit temporary tables. There are existing mechanisms for implicity temporary tables which remain unchanged.

tiancaiamao · 2021-06-03T09:35:02Z

@tiancaiamao As discussed. There are two settings which apply in MySQL 5.7 here:
* [tmp_table_size](https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_tmp_table_size)

* [max_heap_table_size](https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_max_heap_table_size)
They are used together, so the smaller limit is what applies to in-memory temporary tables. These limits are enforced per-temporary table, which is the same semantic as what is used in this PR. The buffer for temptable_max_ram in MySQL 8.0 is a global buffer pool shared by all connections (as an administrator I prefer this behavior, as it's easier to plan provisioning of space for temporary queries). I think it's best to avoid this naming convention to prevent confusion.

Where tmp_table_size and max_heap_table_size differ from this implementation is that TiDB currently has no spill to disk mechanism. I actually think that is okay. If you create a temporary table in MySQL and you explicitly say the engine=MEMORY, it will also not spill to disk when the threshold is reached. You will instead get a table-is-full error.

The bigger difference is that tmp_table_size and max_heap_table_size apply to both implicit and explicit temporary tables, whereas in TiDB this setting will only apply to explicit temporary tables. There are existing mechanisms for implicity temporary tables which remain unchanged.

So complicated ...

max_heap_table_size is a limitation for user-created MEMORY table, so it works on create table ... () engine = memory?
but it also works on internal in-memory tables, that is to say -- temporary table?

tmp_table_size works on the temporary tables, but its semantic is the spill to disk threshold...

tiancaiamao · 2021-06-03T09:39:58Z

I'll change to use the variable to tmp_table_size.
If we implement the spill to disk feature for the temporary table in development of stage 3,
we can modify it to be totally compatible with MySQL.

For now, just raise an error when data size excess tmp_table_size.
And the scope of tmp_table_size is global + session, just as MySQL.

bb7133 · 2021-06-08T04:01:28Z

/merge cancel

wjhuang2016

Please fix the CI

tiancaiamao · 2021-06-09T09:14:41Z

executor/set_test.go

@@ -896,9 +896,9 @@ func (s *testSuite5) TestValidateSetVar(c *C) {
 	result = tk.MustQuery("select @@tmp_table_size;")
 	result.Check(testkit.Rows("167772161"))

-	tk.MustExec("set @@tmp_table_size=18446744073709551615")
+	tk.MustExec("set @@tmp_table_size=9223372036854775807")


This change is because MySQL set the value to Uint64, while in TiDB it's set to Int64

tiancaiamao · 2021-06-09T09:15:47Z

Please fix the CI

Done, please take a look @wjhuang2016 @djshow832

wjhuang2016 · 2021-06-09T09:25:11Z

/merge

ti-chi-bot · 2021-06-09T09:25:14Z

This pull request has been accepted and is ready to merge.

Commit hash: a6c14f7

tiancaiamao · 2021-06-09T10:37:11Z

/merge

ti-chi-bot · 2021-06-09T10:37:14Z

This pull request has been accepted and is ready to merge.

Commit hash: 754ded5

ti-chi-bot · 2021-06-09T10:39:09Z

@tiancaiamao: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

*: add global system variable temptable_max_ram

023af23

tiancaiamao requested a review from djshow832 May 21, 2021 08:52

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 21, 2021

tiancaiamao mentioned this pull request May 21, 2021

Support temporary table #24169

Closed

89 tasks

github-actions bot added the sig/sql-infra SIG: SQL Infra label May 21, 2021

djshow832 reviewed May 24, 2021

View reviewed changes

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2021

Merge branch 'master' into temptable-max-ram

1f16f31

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 25, 2021

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 25, 2021

wjhuang2016 reviewed May 27, 2021

View reviewed changes

table/tables/tables.go Outdated Show resolved Hide resolved

table/tables/tables.go Show resolved Hide resolved

morgo reviewed May 28, 2021

View reviewed changes

sessionctx/variable/sysvar.go Outdated Show resolved Hide resolved

Update sessionctx/variable/sysvar.go

ad60664

Co-authored-by: Morgan Tocker <[email protected]>

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2021

Merge branch 'master' into temptable-max-ram

0f7230f

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2021

use tidb_temptable_max_ram as temptable_max_ram is deprecated in mysql8

06e1dac

tiancaiamao changed the title *: add global system variable temptable_max_ram *: add global system variable tidb_temptable_max_ram Jun 3, 2021

wjhuang2016 reviewed Jun 3, 2021

View reviewed changes

table/tables/tables.go Outdated Show resolved Hide resolved

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 3, 2021

ti-chi-bot added 4 commits June 7, 2021 20:28

Merge branch 'master' into temptable-max-ram

72ed929

Merge branch 'master' into temptable-max-ram

488b007

Merge branch 'master' into temptable-max-ram

745ba41

Merge branch 'master' into temptable-max-ram

b8809cf

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 8, 2021

wjhuang2016 reviewed Jun 8, 2021

View reviewed changes

go fmt

a6c14f7

tiancaiamao requested a review from a team as a code owner June 9, 2021 09:09

tiancaiamao requested review from XuHuaiyu and removed request for a team June 9, 2021 09:09

tiancaiamao commented Jun 9, 2021

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 9, 2021

ti-chi-bot and others added 2 commits June 9, 2021 17:25

Merge branch 'master' into temptable-max-ram

d29dbab

error check

754ded5

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 9, 2021

github-actions bot added the sig/execution SIG execution label Jun 9, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 9, 2021

Merge branch 'master' into temptable-max-ram

146617f

ti-chi-bot merged commit cbb6f4e into pingcap:master Jun 9, 2021

tiancaiamao deleted the temptable-max-ram branch June 18, 2021 14:04

tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Jun 23, 2021

*: add global system variable tmp_table_size (pingcap#24827)

727a217

djshow832 mentioned this pull request Oct 9, 2021

Memory limit of temporary tables should use max_heap_table_size instead of tmp_table_size #28691

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add global system variable `tmp_table_size` #24827

*: add global system variable `tmp_table_size` #24827

tiancaiamao commented May 21, 2021 •

edited

Loading

tiancaiamao commented May 21, 2021

djshow832 May 24, 2021

morgo May 24, 2021

morgo May 28, 2021

tiancaiamao May 28, 2021

djshow832 commented May 25, 2021

djshow832 commented May 25, 2021

tisonkun commented Jun 1, 2021

djshow832 commented Jun 2, 2021

tiancaiamao commented Jun 3, 2021

wjhuang2016 left a comment

morgo commented Jun 3, 2021

tiancaiamao commented Jun 3, 2021

tiancaiamao commented Jun 3, 2021

bb7133 commented Jun 8, 2021

wjhuang2016 left a comment

tiancaiamao Jun 9, 2021

tiancaiamao commented Jun 9, 2021

wjhuang2016 commented Jun 9, 2021

ti-chi-bot commented Jun 9, 2021

tiancaiamao commented Jun 9, 2021

ti-chi-bot commented Jun 9, 2021

ti-chi-bot commented Jun 9, 2021

*: add global system variable tmp_table_size #24827

*: add global system variable tmp_table_size #24827

Conversation

tiancaiamao commented May 21, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

tiancaiamao commented May 21, 2021

djshow832 May 24, 2021

Choose a reason for hiding this comment

morgo May 24, 2021

Choose a reason for hiding this comment

morgo May 28, 2021

Choose a reason for hiding this comment

tiancaiamao May 28, 2021

Choose a reason for hiding this comment

djshow832 commented May 25, 2021

djshow832 commented May 25, 2021

tisonkun commented Jun 1, 2021

djshow832 commented Jun 2, 2021

tiancaiamao commented Jun 3, 2021

wjhuang2016 left a comment

Choose a reason for hiding this comment

morgo commented Jun 3, 2021

tiancaiamao commented Jun 3, 2021

tiancaiamao commented Jun 3, 2021

bb7133 commented Jun 8, 2021

wjhuang2016 left a comment

Choose a reason for hiding this comment

tiancaiamao Jun 9, 2021

Choose a reason for hiding this comment

tiancaiamao commented Jun 9, 2021

wjhuang2016 commented Jun 9, 2021

ti-chi-bot commented Jun 9, 2021

tiancaiamao commented Jun 9, 2021

ti-chi-bot commented Jun 9, 2021

ti-chi-bot commented Jun 9, 2021

*: add global system variable `tmp_table_size` #24827

*: add global system variable `tmp_table_size` #24827

tiancaiamao commented May 21, 2021 •

edited

Loading