Setting `wait_timeout` to 0 doesn't take effect

[djshow832]

Bug Report

After setting wait_timeout to a non-zero value and then setting it back to 0, the connection will still timeout.

func (p *packetIO) readOnePacket() ([]byte, error) {
	var header [4]byte
	if p.readTimeout > 0 {
		if err := p.bufReadConn.SetReadDeadline(time.Now().Add(p.readTimeout)); err != nil {
			return nil, err
		}
	}
...

In this function, setting wait_timeout to 0 won't go into SetReadDeadline, so it doesn't take effect.

1. Minimal reproduce step (Required)

            Statement stmt = conn.createStatement();
            stmt.execute("set @@wait_timeout=1");
            stmt.execute("set @@wait_timeout=0");
            Thread.sleep(3000);
            ResultSet rs = stmt.executeQuery("select 1");

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

The query is executed successfully.

3. What did you see instead (Required)

It reports CommunicationsException.

4. What is your TiDB version? (Required)

v3.0.17

[Howie59]

/assign

[morgo]

In MySQL, the minimum value is actually 1 not zero:

mysql [localhost:8023] {msandbox} (performance_schema) > select * from variables_info where variable_name='wait_timeout';
+---------------+-----------------+---------------+-----------+-----------+----------+----------+----------+
| VARIABLE_NAME | VARIABLE_SOURCE | VARIABLE_PATH | MIN_VALUE | MAX_VALUE | SET_TIME | SET_USER | SET_HOST |
+---------------+-----------------+---------------+-----------+-----------+----------+----------+----------+
| wait_timeout  | COMPILED        |               | 1         | 31536000  | NULL     | NULL     | NULL     |
+---------------+-----------------+---------------+-----------+-----------+----------+----------+----------+
1 row in set (0.00 sec)

Compared to TiDB:

        {Scope: ScopeGlobal | ScopeSession, Name: WaitTimeout, Value: strconv.FormatInt(DefWaitTimeout, 10), Type: TypeUnsigned, MinValue: 0, MaxValue: 31536000, AutoConvertOutOfRange: true},

[djshow832]

Yes, I know. I assume the reason why TiDB is different is that TiDB doesn't support wait_timeout before v3.0, and to be compatible with the older versions, TiDB must support unlimited and make it default. @morgo

[morgo]

OK. I believe @SunRunAway would like to change the default for wait_timeout (the MySQL default is quite ridiculous, and changes in Aurora/RDS). Maybe we can handle it at the same time?

[SunRunAway]

I think they're different things and this bug should be fixed or users cannot revert the wait_timeout setting back.