From f9b1e5223e1c4700fc6fc6bf4f462571a1af47da Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 14:38:37 +0800 Subject: [PATCH 1/8] Add mysql.Context for the compatibility before go1.7 --- connection.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/connection.go b/connection.go index 08e5fadeb..fdc6b553e 100644 --- a/connection.go +++ b/connection.go @@ -17,6 +17,11 @@ import ( "time" ) +// Define mysql.Context for the compatibility before go1.7. +type Context interface { + Deadline() (deadline time.Time, ok bool) +} + type mysqlConn struct { buf buffer netConn net.Conn @@ -31,6 +36,7 @@ type mysqlConn struct { sequence uint8 parseTime bool strict bool + ctx Context } // Handles parameters set in DSN after the connection is established @@ -389,3 +395,16 @@ func (mc *mysqlConn) getSystemVar(name string) ([]byte, error) { } return nil, err } + +// Make all others get the context from the mysqlConn instead of copying it as +// a member of struct everywhere. +type ContextWrapper struct { + mc *mysqlConn +} + +func (wrapper *ContextWrapper) Deadline() (time.Time, bool) { + if wrapper.mc.ctx == nil { + return time.Time{}, false + } + return wrapper.mc.ctx.Deadline() +} From 63fc0b2ae1c499207b0e6774921598e385f47c35 Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 14:54:04 +0800 Subject: [PATCH 2/8] Compare readTimeout with Context.Deadline() --- buffer.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/buffer.go b/buffer.go index 2001feacd..694a78c90 100644 --- a/buffer.go +++ b/buffer.go @@ -22,18 +22,25 @@ const defaultBufSize = 4096 // The buffer is similar to bufio.Reader / Writer but zero-copy-ish // Also highly optimized for this particular use case. type buffer struct { - buf []byte + buf []byte + idx int + length int + nc net.Conn - idx int - length int timeout time.Duration + ctx Context } func newBuffer(nc net.Conn) buffer { + return newBufferContext(nc, nil) +} + +func newBufferContext(nc net.Conn, ctx Context) buffer { var b [defaultBufSize]byte return buffer{ buf: b[:], nc: nc, + ctx: ctx, } } @@ -60,7 +67,15 @@ func (b *buffer) fill(need int) error { for { if b.timeout > 0 { - if err := b.nc.SetReadDeadline(time.Now().Add(b.timeout)); err != nil { + deadline := time.Now().Add(b.timeout) + if b.ctx != nil { + if ctxDeadline, ok := b.ctx.Deadline(); ok { + if !ctxDeadline.IsZero() && ctxDeadline.Before(deadline) { + deadline = ctxDeadline + } + } + } + if err := b.nc.SetReadDeadline(deadline); err != nil { return err } } From b6c6f032c2451c3ec98ada8c774a3fc87493f6e1 Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 14:54:36 +0800 Subject: [PATCH 3/8] Compare writeTimeout with Context.Deadline() --- packets.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packets.go b/packets.go index 41b4d3d55..5f9f5e579 100644 --- a/packets.go +++ b/packets.go @@ -107,7 +107,15 @@ func (mc *mysqlConn) writePacket(data []byte) error { // Write packet if mc.writeTimeout > 0 { - if err := mc.netConn.SetWriteDeadline(time.Now().Add(mc.writeTimeout)); err != nil { + deadline := time.Now().Add(mc.writeTimeout) + if mc.ctx != nil { + if ctxDeadline, ok := mc.ctx.Deadline(); ok { + if !ctxDeadline.IsZero() && ctxDeadline.Before(deadline) { + deadline = ctxDeadline + } + } + } + if err := mc.netConn.SetWriteDeadline(deadline); err != nil { return err } } From 0439f3e38cba0c6af15e3f46d038ee0d63311e08 Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 14:59:33 +0800 Subject: [PATCH 4/8] Make the buffer get the context from mysqlConn --- driver.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/driver.go b/driver.go index e51d98a3c..e7eb5ccb4 100644 --- a/driver.go +++ b/driver.go @@ -81,7 +81,8 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { } } - mc.buf = newBuffer(mc.netConn) + // Make the buffer get the context from mysqlConn. + mc.buf = newBufferContext(mc.netConn, &ContextWrapper{mc: mc}) // Set I/O timeouts mc.buf.timeout = mc.cfg.ReadTimeout From 231b0fb0d1841b5400b856b634b5de050a2cac23 Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 16:20:33 +0800 Subject: [PATCH 5/8] Support driver.Pinger --- connection_go18.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 connection_go18.go diff --git a/connection_go18.go b/connection_go18.go new file mode 100644 index 000000000..8df076ef5 --- /dev/null +++ b/connection_go18.go @@ -0,0 +1,88 @@ +// +build go1.8 + +// Go MySQL Driver - A MySQL-Driver for Go's database/sql package +// +// Copyright 2017 The Go-MySQL-Driver Authors. All rights reserved. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +package mysql + +import ( + "context" + "database/sql/driver" + "net" + "time" +) + +func (mc *mysqlConn) Ping(ctx context.Context) error { + var err error + + if ctx == nil { + return mc.pingImpl() + } + mc.ctx = ctx + defer func() { + mc.ctx = nil + }() + + if deadline, ok := ctx.Deadline(); ok { + if err = mc.netConn.SetDeadline(deadline); err != nil { + return err + } + } + + if ctx.Done() == nil { + return mc.pingImpl() + } + + result := make(chan error) + go func() { + result <- mc.pingImpl() + }() + + select { + case <-ctx.Done(): + // Because buffer.fill() and mysqlConn.writePacket() may overwrite the + // deadline of read/write again and again in the above goroutine, + // it has to use a loop to make sure SetDeadline() works. + UpdateDeadlineLoop: + for { + // Copy it as a local variable because mc.netConn may be closed and + // assigned nil in the above goroutine. + netConn := mc.netConn + if netConn != nil { + errDeadline := netConn.SetDeadline(time.Now()) + errLog.Print(errDeadline) + } + select { + case <-time.After(200 * time.Millisecond): + // Prevent from leaking the above goroutine. + case err = <-result: + break UpdateDeadlineLoop + } + } + case err = <-result: + } + + if netErr, ok := err.(net.Error); ok { + // We don't know where it timed out and it may leave some redundant data + // in the connection so make it to be closed by DB.puConn() of the + // caller. + if netErr.Timeout() { + return driver.ErrBadConn + } + } + return err +} + +func (mc *mysqlConn) pingImpl() error { + if err := mc.writeCommandPacket(comPing); err != nil { + return err + } + + _, err := mc.readResultOK() + return err +} From ab45fbbe7f66f9c0c79ba96c21ca50b58569ceea Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 19:11:22 +0800 Subject: [PATCH 6/8] Print errDeadline only if it is not nil --- connection_go18.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/connection_go18.go b/connection_go18.go index 8df076ef5..6d91d1349 100644 --- a/connection_go18.go +++ b/connection_go18.go @@ -55,7 +55,9 @@ func (mc *mysqlConn) Ping(ctx context.Context) error { netConn := mc.netConn if netConn != nil { errDeadline := netConn.SetDeadline(time.Now()) - errLog.Print(errDeadline) + if errDeadline != nil { + errLog.Print(errDeadline) + } } select { case <-time.After(200 * time.Millisecond): From 3e3200ecfe1b0ab2e88cb7970c7d5790503fa12f Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 19:39:46 +0800 Subject: [PATCH 7/8] Add test cases for driver.Pinger --- connection_go18_test.go | 350 ++++++++++++++++++++++++++++++++++++++++ connection_test.go | 73 +++++++++ 2 files changed, 423 insertions(+) create mode 100644 connection_go18_test.go diff --git a/connection_go18_test.go b/connection_go18_test.go new file mode 100644 index 000000000..a800e6a7d --- /dev/null +++ b/connection_go18_test.go @@ -0,0 +1,350 @@ +// +build go1.8 + +// Go MySQL Driver - A MySQL-Driver for Go's database/sql package +// +// Copyright 2017 The Go-MySQL-Driver Authors. All rights reserved. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +package mysql + +import ( + "context" + "database/sql/driver" + "testing" + "time" +) + +func TestPingNilContext(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + err = pinger.Ping(nil) + if err != nil { + t.Fatalf("err must be nil. err(%s)", err) + } +} + +func TestPingBackgroundContext(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + err = pinger.Ping(context.Background()) + if err != nil { + t.Fatalf("err must be nil. err(%s)", err) + } +} + +func TestPingMySqlCtx(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + err = pinger.Ping(context.Background()) + if err != nil { + t.Fatalf("err must be nil. err(%s)", err) + } + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + + if mc.ctx != nil { + t.Fatalf("mc.ctx must be nil after Ping(). mc.ctx(%s)", mc.ctx) + } +} + +func TestPingCtxDone(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestPingCtxDoneIoWriteTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.writeTimeout, err = time.ParseDuration("1ns") + if !ok { + t.Fatalf("err(%s)", err) + } + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestPingCtxDoneIoReadTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.buf.timeout, err = time.ParseDuration("1ns") + if !ok { + t.Fatalf("err(%s)", err) + } + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestPingCtxDeadline(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, _ := context.WithTimeout(context.Background(), 0) + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestPingCtxDeadlineAfterIoWriteTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.writeTimeout, err = time.ParseDuration("1ns") + if !ok { + t.Fatalf("err(%s)", err) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, _ := context.WithTimeout(context.Background(), 1*time.Hour) + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestPingCtxDeadlineBeforeIoWriteTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.writeTimeout, err = time.ParseDuration("1h") + if !ok { + t.Fatalf("err(%s)", err) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, _ := context.WithTimeout(context.Background(), 0) + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestPingCtxDeadlineAfterIoReadTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.buf.timeout, err = time.ParseDuration("1ns") + if !ok { + t.Fatalf("err(%s)", err) + } + + pinger, ok := conn.(driver.Pinger) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Pinger") + } + + ctx, _ := context.WithTimeout(context.Background(), 1*time.Hour) + + err = pinger.Ping(ctx) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} diff --git a/connection_test.go b/connection_test.go index 65325f101..8f4625b15 100644 --- a/connection_test.go +++ b/connection_test.go @@ -11,6 +11,7 @@ package mysql import ( "database/sql/driver" "testing" + "time" ) func TestInterpolateParams(t *testing.T) { @@ -65,3 +66,75 @@ func TestInterpolateParamsPlaceholderInString(t *testing.T) { t.Errorf("Expected err=driver.ErrSkip, got err=%#v, q=%#v", err, q) } } + +func TestIoWriteTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.writeTimeout, err = time.ParseDuration("1ns") + if !ok { + t.Fatalf("err(%s)", err) + } + + execer, ok := conn.(driver.Execer) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Execer") + } + + _, err = execer.Exec("show databases", nil) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} + +func TestIoReadTimeout(t *testing.T) { + var err error + + if !available { + t.Skipf("MySQL server not running on %s", netAddr) + } + + mySqlDriver := MySQLDriver{} + conn, err := mySqlDriver.Open(dsn) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + + mc, ok := conn.(*mysqlConn) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of mysqlConn") + } + mc.buf.timeout, err = time.ParseDuration("1ns") + if !ok { + t.Fatalf("err(%s)", err) + } + + execer, ok := conn.(driver.Execer) + if !ok { + t.Fatalf("It can't type-assert driver.conn is of driver.Execer") + } + + _, err = execer.Exec("show databases", nil) + if err == nil { + return + } + if err != driver.ErrBadConn { + t.Fatalf("err must be driver.ErrBadConn. err(%s)", err) + } +} From 71107203b1f335f33d69e899b009247b37ae8754 Mon Sep 17 00:00:00 2001 From: Steven Huang Date: Wed, 26 Apr 2017 21:27:20 +0800 Subject: [PATCH 8/8] Implement Pinger interface (#505) --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 48d04eb52..900df648a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -48,6 +48,7 @@ Runrioter Wung Soroush Pour Stan Putrya Stanley Gunawan +Steven Huang Xiangyu Hu Xiaobing Jiang Xiuming Chen