*: move config file option oom-action to sysvar #34644

ramanich1 · 2022-05-13T19:09:33Z

What problem does this PR solve?

Issue Number: ref #33769

Problem Summary:

The option oom-action has historically been a config option. But based on requirements from cloud & PM it should instead be a sysvar.

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility
This might be a bit confusing during upgrade for users who have previously set oom-action(most users haven't). It needs to be documented in the release notes that it has been replaced with the system variable tidb_mem_oom_action.

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

The TiDB configuration file option `oom-action` has been replaced by the system variable `tidb_mem_oom_action`. This change makes it easier to modify this setting on a cluster wide basis.

ti-chi-bot · 2022-05-13T19:09:34Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

morgo
tangenta

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

sre-bot · 2022-05-13T19:09:39Z

All committers have signed the CLA.

sre-bot · 2022-05-13T19:10:06Z

Please follow PR Title Format:

pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

*: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

morgo · 2022-05-13T19:45:13Z

/run-check_title

morgo · 2022-05-13T19:46:31Z

Check_dev looks like simple linter issues:

[2022-05-13T19:13:01.230Z] linting
[2022-05-13T19:13:01.230Z]   ✘  https://revive.run/r#exported  comment on exported const OOMActionCancel should be of the form "OOMActionCancel ..."  
[2022-05-13T19:13:01.230Z]   sessionctx/variable/variable.go:78:2
[2022-05-13T19:13:01.230Z] 
[2022-05-13T19:13:01.230Z]   ✘  https://revive.run/r#exported  exported const OOMActionLog should have comment (or a comment on this block) or be unexported  
[2022-05-13T19:13:01.230Z]   sessionctx/variable/variable.go:80:2
[2022-05-13T19:13:01.230Z] 
[2022-05-13T19:13:01.230Z] ✘ 2 problems (2 errors, 0 warnings)
[2022-05-13T19:13:01.230Z] 
[2022-05-13T19:13:01.230Z] Errors:
[2022-05-13T19:13:01.230Z]   2  exported

It looks like there is a test broken as well:

2022-05-13T19:17:20.821Z] ----------------------------------------------------------------------

[2022-05-13T19:17:20.821Z] FAIL: session_legacy_test.go:3123: testSessionSerialSuite.TestCoprocessorOOMAction

[2022-05-13T19:17:20.821Z] 

[2022-05-13T19:17:20.821Z] enable OOM, testcase: keep Order

[2022-05-13T19:17:20.821Z] disable OOM, testcase: keep Order

[2022-05-13T19:17:20.821Z] session_legacy_test.go:3196:

[2022-05-13T19:17:20.821Z]     disableOOM(tk, testcase.name, testcase.sql)

[2022-05-13T19:17:20.821Z] session_legacy_test.go:3184:

[2022-05-13T19:17:20.821Z]     c.Assert(err, NotNil)

[2022-05-13T19:17:20.821Z] ... value = nil

[2022-05-13T19:17:20.821Z]

sre-bot · 2022-05-16T19:28:48Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/e2e97ecd74a05fc60bf08ae75054870a946bf11b

tangenta · 2022-05-17T07:06:55Z

/merge

ti-chi-bot · 2022-05-17T07:06:58Z

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

Commit hash: 9db3590badd97e5229c19f9bcc9331b05ce3550f

sessionctx/variable/sysvar.go

bb7133 · 2022-05-18T02:49:20Z

/unhold

bb7133 · 2022-05-18T02:49:24Z

/merge

tangenta · 2022-05-18T10:19:41Z

Reopen it to wake "license/cla" up.

bb7133 · 2022-05-18T12:56:52Z

/merge

morgo · 2022-05-18T21:01:40Z

/merge

ti-chi-bot · 2022-05-18T21:01:43Z

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

Commit hash: e2e97ec

sre-bot · 2022-05-18T21:43:20Z

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci/integration-cdc-test	🟥 failed 1, success 33, total 34	22 min	New failing
idc-jenkins-ci-tidb/integration-common-test	✅ all 11 tests passed	10 min	Fixed
idc-jenkins-ci-tidb/common-test	🟢 all 12 tests passed	5 min 12 sec	Existing passed
idc-jenkins-ci-tidb/integration-ddl-test	🟢 all 6 tests passed	5 min 2 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	5 min 2 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	4 min 51 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	4 min 22 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	3 min 32 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 53 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

ramanich1 requested a review from a team as a code owner May 13, 2022 19:09

ti-chi-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 13, 2022

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

morgo changed the title ~~move config file option oom-action to sysvar~~ *: move config file option oom-action to sysvar May 13, 2022

morgo mentioned this pull request May 13, 2022

*: move config file option tidb_enable_auto_analyze to sysvar #34643

Merged

12 tasks

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

morgo mentioned this pull request May 16, 2022

Convert several config options to Global sysvars #33769

Open

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 16, 2022

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 16, 2022

morgo self-requested a review May 16, 2022 19:57

morgo approved these changes May 17, 2022

View reviewed changes

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

tangenta approved these changes May 17, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 17, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 17, 2022

bb7133 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 17, 2022

bb7133 reviewed May 17, 2022

View reviewed changes

sessionctx/variable/sysvar.go Show resolved Hide resolved

morgo mentioned this pull request May 17, 2022

session: improve bootstrap code #34755

Merged

12 tasks

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 18, 2022

tangenta closed this May 18, 2022

tangenta reopened this May 18, 2022

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

ramanich1 force-pushed the oomaction branch from d7e60fc to 4114d6e Compare May 18, 2022 17:43

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed status/can-merge Indicates a PR has been approved by a committer. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 18, 2022

OOM action changes

03652d5

ramanich1 force-pushed the oomaction branch from 125684f to 03652d5 Compare May 18, 2022 20:36

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 18, 2022

Update tidb_vars.go

e2e97ec

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 18, 2022

ti-chi-bot merged commit a961485 into pingcap:master May 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: move config file option oom-action to sysvar #34644

*: move config file option oom-action to sysvar #34644

ramanich1 commented May 13, 2022

ti-chi-bot commented May 13, 2022 •

edited

Loading

sre-bot commented May 13, 2022 •

edited

Loading

sre-bot commented May 13, 2022

morgo commented May 13, 2022

morgo commented May 13, 2022

sre-bot commented May 16, 2022 •

edited

Loading

tangenta commented May 17, 2022

ti-chi-bot commented May 17, 2022

bb7133 commented May 18, 2022

bb7133 commented May 18, 2022

tangenta commented May 18, 2022

bb7133 commented May 18, 2022

morgo commented May 18, 2022

ti-chi-bot commented May 18, 2022

sre-bot commented May 18, 2022

*: move config file option oom-action to sysvar #34644

*: move config file option oom-action to sysvar #34644

Conversation

ramanich1 commented May 13, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented May 13, 2022 • edited Loading

sre-bot commented May 13, 2022 • edited Loading

sre-bot commented May 13, 2022

morgo commented May 13, 2022

morgo commented May 13, 2022

sre-bot commented May 16, 2022 • edited Loading

tangenta commented May 17, 2022

ti-chi-bot commented May 17, 2022

bb7133 commented May 18, 2022

bb7133 commented May 18, 2022

tangenta commented May 18, 2022

bb7133 commented May 18, 2022

morgo commented May 18, 2022

ti-chi-bot commented May 18, 2022

sre-bot commented May 18, 2022

TiDB MergeCI notify

ti-chi-bot commented May 13, 2022 •

edited

Loading

sre-bot commented May 13, 2022 •

edited

Loading

sre-bot commented May 16, 2022 •

edited

Loading