*: Move tikv gc configuration to sysvars

[morgo]

What problem does this PR solve?

Fixes #20655 (via documentation/new process. Doesn't fix it as originally intended).

Workaround for #22145 (more permanent solution required)

Problem Summary:

Currently the garbage collection uses a table mysql.tidb for configuration and state. This causes a couple of problems:

1. When users update the table, validation can not be performed on the values. This can lead to strange behaviors as when reading the values, interpretation has to be performed. It would be nicer for users to get immediate feedback as well (which sysvars provide).
2. It requires users to be able to read the mysql.tidb table in order to run programs like dumpling, which must extend the garbage collection window. Because there is no row-level security option, this means that all values in the table need to be visible. The dbaas team has requested that this internal state not be exposed to users because of the risk of changes degrading the system.

What is changed and how it works?

What's Changed:

The gc code has been converted to use sysvars, and use the sysvar's validation system (which removes some code). The option for tikv_gc_auto_concurrency has been merged with tikv_gc_concurrency, because MySQL has an "autovalue" semantic of -1 that fits nicely here.

The state remains in mysql.tidb.

When updating a sysvar, the corresponding mysql.tidb value is updated. When reading a sysvar, the read is redirected to mysql.tidb if it is one of the tikv values.

Related changes

• This does not explicitly need support by dumpling/BR (because the old methods continue to work), but they should eventually transition to use sysvars instead of tables. This will be required in the case of dumpling backing up a dbaas installation once the mysql.tidb table is locked down.
• The docs PR is Update garbage collection docs docs#4552

Check List

Tests

• Unit test
• Manual test (add detailed scripts or steps below)

Side effects

• This is backwards compatible with previous usage, but it adds code complexity and possibly impacts performance for the command SHOW GLOBAL VARIABLES / getting all global sysvars.
• It is recommended that the mysql.tidb usage be deprecated, and removed in a future version.

Release note

• TiKV garbage collection settings are now configurable as system variables. For example: SET GLOBAL tikv_gc_enable = FALSE. The previous usage of manually updating the mysql.tidb table is deprecated, but continues to work for now.

[youjiali1995]

A related issue: #20655

[morgo]

A related issue: #20655

Thanks, this looks like it fixes it. So I have updated the description.

[morgo]

/run-all-tests

[tennix]

I think this might have an impact on downstream tools such as BR, tidb-operator which uses the old tikv gc lifetime configuration. /cc @DanielZhangQD @3pointer

[youjiali1995]

I think this might have an impact on downstream tools such as BR, tidb-operator which uses the old tikv gc lifetime configuration. /cc @DanielZhangQD @3pointer

Yes. It breaks backward compatibility. It's the reason why we haven't fix #20655..

[youjiali1995]

@MyonKeminta proposes that we still save GC configurations in mysql.tidb and provide a new way to change it through sysvars. mysql.tidb is for backward compatibility and sysvars is for validation check. @morgo @tennix

@scsldb PTAL.

[morgo]

@MyonKeminta proposes that we still save GC configurations in mysql.tidb and provide a new way to change it through sysvars. mysql.tidb is for backward compatibility and sysvars is for validation check. @morgo @tennix

That was my original plan, but the hard part to implement is that updates to mysql.tidb will not update the sysvar value. So reading @@tikv_* could show a stale value. Because TiDB does not have a trigger mechanism, we would need to implement it from the sysvar side (make reading some sysvars trigger a callback function). It's not impossible, but it does add complexity.

Another idea is that we could make the mysql.tidb table read-only by default (maybe protected by a setting). This will at least break any applications in a stronger way if they attempt to use the old mechanism. In most cases, I think the programs that change these values are released by us. The most common risk (and it is a real problem) is a user using an outdated dumpling client on a newer server.

[youjiali1995]

That was my original plan, but the hard part to implement is that updates to mysql.tidb will not update the sysvar value. So reading @@tikv_* could show a stale value.

GC related sysvars use the data in mysql.tidb too. Updating these sysvars is updating mysql.tidb. Reading it is reading mysql.tidb.

[morgo]

GC related sysvars use the data in mysql.tidb too. Updating these sysvars is updating mysql.tidb. Reading it is reading mysql.tidb.

The last sentence is the part that needs to be implemented, and what I had meant by this:

(make reading some sysvars trigger a callback function)

I will take a look at doing this. Reading the sysvar triggers a read to mysql.tidb.

[MyonKeminta]

Thanks!
Since this is a new feature, it's recommended to write a design document for it. So please consider write one, although it might be simple. If you don't have enough time, It's also ok to leave the document to me.

[MyonKeminta]

LGTM

[MyonKeminta]

This still doesn't looks safe enough 😕 I think we need better way to do this in the future

[morgo]

Yes, I agree. I searched and couldn't find anything reliable that could be imported in util or in a library. See golang/go#18478 for example.

[morgo]

I have removed the tidb_gc_mode based on reading the docs in this PR here: pingcap/docs#4485

It is fine to stay in the mysql.tidb table, but it can be really confusing for users if they set something and it has no effect. If it is no longer used there is no point in introducing a variable for it, since there is no compatibility argument.

PTAL @MyonKeminta @youjiali1995

[youjiali1995]

/merge

[ti-srebot]

/run-all-tests