github / gh-ost

GitHub's Online Schema-migration Tool for MySQL
MIT License
12.43k stars 1.26k forks source link

port check on @@global.port prevents execution on 1st generation GCP instances. #632

Open jlorberblatt opened 6 years ago

jlorberblatt commented 6 years ago

When executing against a gcp 1st generation sql instance gh-ost fails with the following error:

[root@centos1 gh-ost]# ./bin/gh-ost --user="testuser" --ask-pass --table="test_table" --database="test" --host="x.x.x.x" --alter="engine=innodb"  --port="3306" --allow-on-master --execute --initially-drop-old-table --initially-drop-ghost-table --verbose --ok-to-drop-table
Password:
2018-08-29 08:55:39 INFO starting gh-ost d2726c77f86cb65e25bce5c0ac5fe3fa0c997488
2018-08-29 08:55:39 INFO Migrating `test`.`test_table`
2018-08-29 08:55:46 INFO Tearing down inspector
2018-08-29 08:55:46 FATAL Unexpected database port reported: 0

The value of @@global.port is checked at two points in the code:

[root@centos1 go]# grep -rn @@global.port *
base/utils.go:81:               portQuery := `select @@global.port`
mysql/utils.go:177:     err = db.QueryRow(`select @@global.hostname, @@global.port`).Scan(&instanceKey.Hostname, &instanceKey.Port)

the following command demonstrates the port value internal to mysql:

mysql> select @@global.port;
+---------------+
| @@global.port |
+---------------+
|             0 |
+---------------+
1 row in set (0.08 sec)

To ignore this value and use the one supplied or default to 3306 which is already being done for other cloud providers I supplied a vendor agnostic command line parameter -skip-port-check in my branch: https://github.com/jlorberblatt/gh-ost/commit/1f7b4087f4a7c1136939427e10c93f2876809970 which I have tested and works.

shlomi-noach commented 6 years ago

@jlorberblatt thank you!

The existing --aliyun-rds is a declarative flag, which does not necessarily imply skipping the port check (related: https://github.com/github/gh-ost/issues/631). Perhaps tomorrow Aliyun RDS would imply different checks. So I would not want to couple --skip-port-check with --aliyun-rds.

Instead, I suggest adding a --gcp flag, which incidentally will do same thing as --aliyun-rds does.

shlomi-noach commented 6 years ago

Addressed by #634 . @jlorberblatt would you care to test #634 on GCP?

fipar commented 6 years ago

Verified that this works as expected.

shlomi-noach commented 6 years ago

Closed by #634