Changing the code here does not take effect actually, because UpdateTableStatsLoop is only called on the bootstrap stage of TiDB if you check the code in BootstrapSession().

This is fine before because when it is changed in the configuration file, TiDB always needs to restart and the BootstrapSession() is called, but making it a sysvar means it the value can be changed dynamically, so:

• When it was FALSE and set to TRUE, the auto-analyze worker will not be invoked.
• When it was TRUE and set to FALSE, the auto-analyze worker will still be there.

You can easily verify it by, for example, adding a log in HandleAutoAnalyze().

So my suggestion is removing the if here:

// always runs the loop
do.wg.Run(func() { do.autoAnalyzeWorker(owner) })

And moving if to here like:

		select {
		case <-analyzeTicker.C:
			if owner.IsOwner() && variable.RunAutoAnalyze.Load() {
				statsHandle.HandleAutoAnalyze(do.InfoSchema())
			}
		case <-do.exit:
			return
		}

So that the sysvar takes effect really.

@chrysan Please let me know if you have any comment on it, thanks!

LGTM

Since the default value of RunAutoAnalyze is true, I think simply removing it may cause some trouble. @chrysan PTAL.

It's false in the test suite. See the change in session/bootstrap.go:2012.

Prefer to keep the variable set in ut or at least an assertion to make the ut safe no matter what has been changed in variable module.

Agreed, an explicit update for UT is more solid. @Alkaagr81 could you update this?

The problem with setting it in the test is:

• If you update the atomic directly, it can get overwritten when the sysvar cache is updated. This is because the sysvar cache calls the SetGlobal func with the value from the mysql.global_variables.
• To safely change it, tk.MustExec("set global sysvar=x") must be used. But tk is not available in all of the tests. We could add it, but it's quite a bit more code to set up each time.

This is why it uses the bootstrap to disable auto-analyze instead.

We can however add an assertion that runautoanalyze is false. Reading the atomic value is no problem.

I see, an alternative way is to create it as another 'enhancement' issue as a backlog, then we could make this PR delivered with the next version(v6.1)

Forked to #34792