-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enhancement:does it support aliyun rds? #470
Comments
@dikang123 any more input is appreciated. What is the table you're trying to convert? Which |
original table structure:
mysql version:5.6.16-log alter statement:
|
@dikang123 can I get the log preceeding that fatal message please? @siddontang do you have any insight into aliyun rds? |
@shlomi-noach the only thing I got after typing the command was the error:
is there some sort of debug mode? |
@dikang123 please run with |
I have not used Aliyun RDS before. We should know the executing query which causes the panic. |
@shlomi-noach
|
pro tip: use three backticks for multi line code formatting. See how I've edited your comment. |
@siddontang not even you code -- thank you! |
@dikang123 the error is with reading The error message unfortunately doesn't make it clear where this happened exactly, but by elimination I suspect this is the result of executing |
@shlomi-noach seems like the port is NULL ! [root@proxy ~]# mysql -uxxx -pxxx-hxxx-e "select @@global.version" |
@dikang123 thank you. Your task is to find out why |
@shlomi-noach yeah, I agree about that. There exists some limitations to make the global variable 'port' a null value! Is there some method to ignore or bypass this issue? |
@dikang123, heh, you tell me 😄 If you can please run a quick investigation and let me know what BTW, what is the output of |
|
doesn't even mention |
aliyun RDS got lots of limits:
should I submit a PR to solve this issue? any suggestions? |
@exherb how do you mean "no replicate"?
I'm unsure how far I will pursue this, this really depends on what aliyun does or doesn't support. |
aliyun RDS replicate is readonly, so it's impossible to migrate on replicate or test on repliacate (migrate on master is possible).
Is there anyway to skip hostname and port validating? So I could dig more informations. |
partially. You may use |
@exherb I ran the test on the master. |
@dikang123 have you found a solution to this? |
@dikang123 @exherb This problem is caused by @@global.port = NULL and gh-ost read from Aliyun RDS and try to covert a NULL to int... We will fix it or provide some interface to fix that. Thanks for @shlomi-noach 's analysis |
@zhangxiaojian any progress? |
@dikang123 Considering rds's security, we can't expose the port and hostname for now. so We fixed the problem by changing gh-ost's source code, please contact me if you need it . email : zj118228@alibaba-inc.com |
@zhangxiaojian why not submit a PR? |
@zhangxiaojian thank you, I send you an email by my Work E-mail : zhukang@dianwoba.com |
I'll be grateful if you can share the code in public. |
@shlomi-noach @exherb @dikang123 That's really not a hard work to fix it. The problem is Aliyun RDS return ''NULL" for the variables @@global.port and @@global.hostname , the port is for check and the hostname is for log output. I try to avoid getting them and gh-ost still works. After an internal discussion, we still can't expose the port and hostname because rds's whole architecture and security, so if gh-ost can week the check some way ? I'm looking forward to make gh-ost work for Aliyun rds, and that's my diff code, and works with --assume-master-host --assume-rbr --allow-on-master diff --git a/go/base/utils.go b/go/base/utils.go
index 727bc57..f391a5a 100644
--- a/go/base/utils.go
+++ b/go/base/utils.go
@@ -65,12 +65,14 @@ func StringContainsAll(s string, substrings ...string) bool {
}
func ValidateConnection(db *gosql.DB, connectionConfig *mysql.ConnectionConfig) (string, error) {
- query := `select @@global.port, @@global.version`
+ //query := `select @@global.port, @@global.version`
+ query := `select @@global.version`
var port, extraPort int
var version string
- if err := db.QueryRow(query).Scan(&port, &version); err != nil {
+ if err := db.QueryRow(query).Scan(&version); err != nil {
return "", err
}
+ port = connectionConfig.Key.Port
extraPortQuery := `select @@global.extra_port`
if err := db.QueryRow(extraPortQuery).Scan(&extraPort); err != nil {
// swallow this error. not all servers support extra_port
diff --git a/go/mysql/utils.go b/go/mysql/utils.go
index b670921..c8ce521 100644
--- a/go/mysql/utils.go
+++ b/go/mysql/utils.go
@@ -148,7 +148,7 @@ func GetSelfBinlogCoordinates(db *gosql.DB) (selfBinlogCoordinates *BinlogCoordi
// GetInstanceKey reads hostname and port on given DB
func GetInstanceKey(db *gosql.DB) (instanceKey *InstanceKey, err error) {
instanceKey = &InstanceKey{}
- err = db.QueryRow(`select @@global.hostname, @@global.port`).Scan(&instanceKey.Hostname, &instanceKey.Port)
+ //err = db.QueryRow(`select @@global.hostname, @@global.port`).Scan(&instanceKey.Hostname, &instanceKey.Port)
return instanceKey, err
} |
@zhangxiaojian first, thank you! |
@shlomi-noach That's very nice , Let's fix it ! |
Given that #541 is merged, can we close this issue? |
Still got |
@xpol please run with |
I have used all flags suggested in #541:
|
@xpol can you please paste output with |
gh-ost \
--max-load=Threads_running=25 \
--critical-load=Threads_running=1000 \
--chunk-size=1000 \
--max-lag-millis=1500 \
--user="..." \
--password="..." \
--host=....mysql.rds.aliyuncs.com \
--allow-on-master \
--database="..." \
--table="..." \
--verbose \
--alter="ADD COLUMN user_id int AFTER id" \
--allow-on-master \
--assume-rbr \
--assume-master-host \
--aliyun-rds \
--debug \
--stack
|
Wow this error is lost in the logs. It is generated somewhere that is not listed. Are you sure there's |
(yes there is |
Unrelated: no need to define |
Removed gh-ost \
--max-load=Threads_running=25 \
--critical-load=Threads_running=1000 \
--chunk-size=1000 \
--max-lag-millis=1500 \
--user="..." \
--password="..." \
--host=....mysql.rds.aliyuncs.com \
--allow-on-master \
--database="pap" \
--table="mytable" \
--verbose \
--alter="ADD COLUMN user_id int AFTER id" \
--allow-on-master \
--assume-rbr \
--aliyun-rds \
--debug \
--stack
|
I guess i should add |
@xpol what's your rds instance version ? 5.6 or 5.7 ? |
Yes, need --assume-master-host following the instance name. |
By the way, @shlomi-noach I have written an article about gh-ost in our teem blog, http://mysql.taobao.org/monthly/2018/05/ , sorry in chinese.. I think we can update the gh-ost's doc to show how it works now. What's your opinion ? |
Yes please! |
@zhangxiaojian MySQL: 5.6 |
should I close this issue? @shlomi-noach @xpol @zhangxiaojian |
Thank you all! Issue is closed and aliyun RDS is supported. |
aliyun rds is a version of amazon rds in China:aliyun rds.
I have a test,but return some sort of error:
2017-08-15 16:40:38 FATAL sql: Scan error on column index 0: converting driver.Value type <nil> ("<nil>") to a int: invalid syntax
The text was updated successfully, but these errors were encountered: