Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Concurrent bug in ReadTopologyInstanceBufferable #1000

Open
fengve opened this issue Oct 29, 2019 · 0 comments
Open

Concurrent bug in ReadTopologyInstanceBufferable #1000

fengve opened this issue Oct 29, 2019 · 0 comments
Labels

Comments

@fengve
Copy link
Contributor

fengve commented Oct 29, 2019

Error handling of the ReadTopologyInstanceBufferable function has serious bugs.

Let's look at the code below.

`

{
		waitGroup.Add(1)
		go func() {
			defer waitGroup.Done()
			var dummy string
			// show global status works just as well with 5.6 & 5.7 (5.7 moves variables to performance_schema)
			err = db.QueryRow("show global status like 'Uptime'").Scan(&dummy, &instance.Uptime)

			if err != nil {
				logReadTopologyInstanceError(instanceKey, "show global status like 'Uptime'", err)
			}
		}()
	}

	var mysqlHostname, mysqlReportHost string
	err = db.QueryRow("select @@global.hostname, ifnull(@@global.report_host, ''), @@global.server_id, @@global.version, @@global.version_comment, @@global.read_only, @@global.binlog_format, @@global.log_bin, @@global.log_slave_updates").Scan(
		&mysqlHostname, &mysqlReportHost, &instance.ServerID, &instance.Version, &instance.VersionComment, &instance.ReadOnly, &instance.Binlog_format, &instance.LogBinEnabled, &instance.LogSlaveUpdatesEnabled)
	if err != nil {
		goto Cleanup
	}
	partialSuccess = true // We at least managed to read something from the server.
	switch strings.ToLower(config.Config.MySQLHostnameResolveMethod) {
	case "none":
		resolvedHostname = instance.Key.Hostname
	case "default", "hostname", "@@hostname":
		resolvedHostname = mysqlHostname
	case "report_host", "@@report_host":
		if mysqlReportHost == "" {
			err = fmt.Errorf("MySQLHostnameResolveMethod configured to use @@report_host but %+v has NULL/empty @@report_host", instanceKey)
			goto Cleanup
		}
		resolvedHostname = mysqlReportHost
	default:
		resolvedHostname = instance.Key.Hostname
	}

`

1、 "MySQLHostnameResolveMethod": "@@report_host" in the orchestrator.conf.json
2、report_host="" in my.cnf

The code will run to
`

    case "report_host", "@@report_host":
		if mysqlReportHost == "" {
			err = fmt.Errorf("MySQLHostnameResolveMethod configured to use @@report_host but %+v has NULL/empty @@report_host", instanceKey)
			goto Cleanup
		}
         .......
       .........

Cleanup:
waitGroup.Wait()

`

The result of code execution might be:
1、err = fmt.Errorf("MySQLHostnameResolveMethod configured to use @@report_host but %+v has NULL/empty @@report_host", instanceKey)
2、err = db.QueryRow("show global status like 'Uptime'").Scan(&dummy, &instance.Uptime)

The err of the goroutine covers the above。

Finally err becomes nil, which is originally fmt.Errorf("MySQLHostnameResolveMethod configured to use @@report_host but %+v has NULL/empty @@report_host", instanceKey)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants