Check configuration

[breezewish]

Currently TiDB does not check most of the configuration. It would be nice to check and warn users when something is not configured correctly / valid.

[jackysp]

@breeswish , can you list the configuration items that need to be checked?

[breezewish]

@jackysp I'm not an expert on this and I'm afraid I can't list them without left. I know at least for tikv-client, the newly introduced grpc time and timeout should be checked, i.e. must not be 0. Also, for existing configurations, the grpc connection count should be definitely > 0 as well.

[jackysp]

The existing grpc connection count was checked at

tidb/tidb-server/main.go

Line 392 in 6dcaeca

if cfg.TiKVClient.GrpcConnectionCount > 0 {

I think we only need to do the same thing in the new pr.

[breezewish]

I think it's not right to silently ignore the wrong configuration and use default values, which hides the error and make it undetectable.

[breezewish]

Also there is a lot of other kind of "durations" in the rest of the config, which is not checked.

[jackysp]

The configuration check has been done once. The principle at the time was to leave as much room as possible for each configuration. If you want to limit more configurations, I think somebody will disagree with you, except me. :)
So I think if you want to check the configurations you added, just do it.

[shenli]

@breeswish We already have validation for the configuration. If 0 is an invalid value for grpc-timeout, I think it is OK to check it.

[kolbe]

Maybe this is not the place to discuss this, but I'm a little concerned about the behavior in TiDB to silently ignore invalid configuration options. For example, I put "status-port" outside of the [status] section of config.toml and was tearing my hair out wondering why it wasn't working before I figured out what I was doing. IMO, tidb-server should simply refuse to start if someone uses a configuration option that doesn't exist. This helps users avoid wasting time (and maybe endangering their data or production environment!) doing something silly like I did, or mis-spelling an option, etc.

[kolbe]

@GregoryIan Morgan suggested I ping you regarding my suggestion that tidb-server (and other components of TiDB Platform) should refuse to start if there are any unrecognized items in the configuration file(s).

[kolbe]

@breeswish we now have support in tidb-server for --config-check and --config-strict:

  -config-check
        check config file validity and exit (default false)
  -config-strict
        enforce config file validity (default false)

I would like to see --config-strict be the default behavior; maybe we can get there in a future release.

I think these items address your original concern, so I'm going to close this issue now, but please re-open it if you have additional questions or ongoing concerns about this topic. I agree it's very important.