in function MySQLDriver.Open: replace errBadConnNoWrite with driver.ErrBadConn for resend while 'bad connection' happenning

[sword2ya]

Description

in function MySQLDriver.Open: replace errBadConnNoWrite with driver.ErrBadConn for resend while 'bad connection' happenning

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

[methane]

When Open() failed, it's not problem error is errBadConnNoWrite or ErrBadConn.
This connection is no more used.

[sword2ya]

When Open() failed, it's not problem error is errBadConnNoWrite or ErrBadConn.
This connection is no more used.

open return ErrBadConn is useful for DB.conn to retry connecting. such as function DB.PingContext

[methane]

open return ErrBadConn is useful for DB.conn to retry connecting. such as function DB.PingContext

Then, you shouldn't use markBadConn().
It returns ErrBadConn when connection is in clean state. (no command is sent yet).

Retry query and retry connecting is totally different problem.

[sword2ya]

open return ErrBadConn is useful for DB.conn to retry connecting. such as function DB.PingContext

Then, you shouldn't use markBadConn().
It returns ErrBadConn when connection is in clean state. (no command is sent yet).

Retry query and retry connecting is totally different problem.

func (db *DB) PingContext(ctx context.Context) error {
	var dc *driverConn
	var err error

	for i := 0; i < maxBadConnRetries; i++ {
		dc, err = db.conn(ctx, cachedOrNewConn)
		if err != driver.ErrBadConn {
			break
		}
	}
	if err == driver.ErrBadConn {
		dc, err = db.conn(ctx, alwaysNewConn)
	}
	if err != nil {
		return err
	}

	return db.pingDC(ctx, dc, dc.releaseConn)
}

like function PingContext(), it will make a new conn to retry connect for us if we return ErrBadConn.
but it won't if we return errBadConnNoWrite.
sometimes users will get the error without retry

[sword2ya]

open return ErrBadConn is useful for DB.conn to retry connecting. such as function DB.PingContext

Then, you shouldn't use markBadConn().
It returns ErrBadConn when connection is in clean state. (no command is sent yet).
Retry query and retry connecting is totally different problem.

func (db *DB) PingContext(ctx context.Context) error {
	var dc *driverConn
	var err error

	for i := 0; i < maxBadConnRetries; i++ {
		dc, err = db.conn(ctx, cachedOrNewConn)
		if err != driver.ErrBadConn {
			break
		}
	}
	if err == driver.ErrBadConn {
		dc, err = db.conn(ctx, alwaysNewConn)
	}
	if err != nil {
		return err
	}

	return db.pingDC(ctx, dc, dc.releaseConn)
}

like function PingContext(), it will make a new conn to retry connect for us if we return ErrBadConn.
but it won't if we return errBadConnNoWrite.
sometimes users will get the error without retry

// PrepareContext creates a prepared statement for later queries or executions.
// Multiple queries or executions may be run concurrently from the
// returned statement.
// The caller must call the statement's Close method
// when the statement is no longer needed.
//
// The provided context is used for the preparation of the statement, not for the
// execution of the statement.
func (db *DB) PrepareContext(ctx context.Context, query string) (*Stmt, error) {
	var stmt *Stmt
	var err error
	for i := 0; i < maxBadConnRetries; i++ {
		stmt, err = db.prepare(ctx, query, cachedOrNewConn)
		if err != driver.ErrBadConn {
			break
		}
	}
	if err == driver.ErrBadConn {
		return db.prepare(ctx, query, alwaysNewConn)
	}
	return stmt, err
}

func (db *DB) prepare(ctx context.Context, query string, strategy connReuseStrategy) (*Stmt, error) {
	// TODO: check if db.driver supports an optional
	// driver.Preparer interface and call that instead, if so,
	// otherwise we make a prepared statement that's bound
	// to a connection, and to execute this prepared statement
	// we either need to use this connection (if it's free), else
	// get a new connection + re-prepare + execute on that one.
	dc, err := db.conn(ctx, strategy)
	if err != nil {
		return nil, err
	}
	return db.prepareDC(ctx, dc, dc.releaseConn, nil, query)
}

and the PrepareContext does the same on Open() return errors

[methane]

You won't understand me.
The point is "when sql.DB should retry Open()"?

markBadConn() returns ErrBadConn when no data is sent in this connection.
It is designed for use in Query() or Exec. Because queries may not be idempotent,
retrying query is dangerous once we send it.

But in case of Open, it's nonsense. We don't send any arbitrary query on it.
See #867.

[sword2ya]

@methane acturally, our team get the problem, it write handshake response failed. and we must do reconnect by ourselves. like this program:

	db, err := sql.Open("mysql", dns)

	for i := 0; i < 2; i++ {
		if err = db.Ping(); err != nil {
			continue
		}
	}
	if err != nil {
		return nil, err
	}
	return db, nil

what's your suggestions?

[methane]

what's your suggestions?

Reconnect by yourselves, if it's needed only for your environment.

If you think it's generally worth for all user of this driver, you must explain the detail of your error.
I don't know what "it write handshake response failed. " means concretely.

[sword2ya]

I don't know what "it write handshake response failed. " means concretely.

it mean the error is returned by writeHandshakeResponsePacket

[methane]

What is "the error"? Traceback? Packet capture? Database version? MySQL or MariaDB?

I think you need to learn how to report issue before sending pull request...