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

"advanced" memory quota variables have no effect #32286

Closed
Tracked by #32679
kolbe opened this issue Feb 11, 2022 · 6 comments · Fixed by #32724
Closed
Tracked by #32679

"advanced" memory quota variables have no effect #32286

kolbe opened this issue Feb 11, 2022 · 6 comments · Fixed by #32724
Labels
severity/moderate sig/execution SIG execution type/bug This issue is a bug.

Comments

@kolbe
Copy link
Contributor

kolbe commented Feb 11, 2022

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

set tidb_mem_quota_hashjoin=0;

2. What did you expect to see? (Required)

This variable is "deprecated". A deprecated variable should still have some effect, especially since the documentation still claims that they have some effect: https://docs.pingcap.com/tidb/dev/configure-memory-usage

3. What did you see instead (Required)

These variables should be removed from the documentation if they have no effect.

The source code claims that these variables "do not take any effect anymore", and there's even a TODO to remove them in 4.1. This warning has been in the code for nearly 2 years. Maybe finally time to remove these entirely?

https://github.com/pingcap/tidb/blob/master/sessionctx/variable/session.go#L1904

4. What is your TiDB version? (Required)

Edition: Community
Git Commit Hash: 55f3b24c1c9f506bd652ef1d162283541e428872
Git Branch: heads/refs/tags/v5.4.0
UTC Build Time: 2022-01-25 08:41:03
GoVersion: go1.16.4
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false
@kolbe kolbe added the type/bug This issue is a bug. label Feb 11, 2022
@morgo morgo assigned morgo and unassigned morgo Feb 11, 2022
@morgo
Copy link
Contributor

morgo commented Feb 11, 2022

This is an easy fix if this is true: we have a feature called "remove sysvars" now: #28931

@yudongusa Can you confirm that we can remove these variables from tidb master?

@yudongusa
Copy link

This is an easy fix if this is true: we have a feature called "remove sysvars" now: #28931

@yudongusa Can you confirm that we can remove these variables from tidb master?

I'll leave this for @XuHuaiyu to double check as this is cap memory usage of hash join operator.

@coderplay
Copy link
Contributor

coderplay commented Feb 11, 2022

@XuHuaiyu Please take a look at this as well: #32287

@coderplay
Copy link
Contributor

@XuHuaiyu so does this one: #32288

@coderplay
Copy link
Contributor

4th issue: #32289

@morgo
Copy link
Contributor

morgo commented Feb 13, 2022

I just rememebered I have a related PR on this: #30848 -- The following system variables are 'known' to not be working correctly: TiDBMemQuotaQuery, TiDBMemQuotaHashJoin, TiDBMemQuotaMergeJoin, TiDBMemQuotaSort, TiDBMemQuotaTopn, TiDBMemQuotaIndexLookupReader.

So it makes sense they should just be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/moderate sig/execution SIG execution type/bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants