From a262126b5c97e5e56559b9f394ae4a994dc19681 Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Sat, 24 Aug 2019 23:49:59 -0500 Subject: [PATCH] Replace IsAlive with IsClosed IsAlive is ambiguous because the connection may be dead and we do not know it. It implies the possibility of a ping. IsClosed is clearer -- it does not promise the connection is alive only that it hasn't been closed. --- conn.go | 17 ++++------------- conn_test.go | 8 ++++---- go.mod | 4 ++-- go.sum | 3 +++ pgxpool/conn.go | 2 +- stdlib/sql.go | 10 +++++----- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/conn.go b/conn.go index 36352d1c..fb94ee8d 100644 --- a/conn.go +++ b/conn.go @@ -57,8 +57,6 @@ type Conn struct { logger Logger logLevel LogLevel - causeOfDeath error - notifications []*pgconn.Notification doneChan chan struct{} @@ -233,12 +231,11 @@ func connect(ctx context.Context, config *ConnConfig) (c *Conn, err error) { // Close closes a connection. It is safe to call Close on a already closed // connection. func (c *Conn) Close(ctx context.Context) error { - if !c.IsAlive() { + if c.IsClosed() { return nil } err := c.pgConn.Close(ctx) - c.causeOfDeath = errors.New("Closed") if c.shouldLog(LogLevelInfo) { c.log(ctx, LogLevelInfo, "closed connection", nil) } @@ -310,12 +307,8 @@ func (c *Conn) WaitForNotification(ctx context.Context) (*pgconn.Notification, e return n, err } -func (c *Conn) IsAlive() bool { - return c.pgConn.IsAlive() -} - -func (c *Conn) CauseOfDeath() error { - return c.causeOfDeath +func (c *Conn) IsClosed() bool { + return c.pgConn.IsClosed() } // Processes messages that are not exclusive to one context such as @@ -362,12 +355,10 @@ func (c *Conn) rxErrorResponse(msg *pgproto3.ErrorResponse) *pgconn.PgError { } func (c *Conn) die(err error) { - if !c.IsAlive() { + if c.IsClosed() { return } - c.causeOfDeath = err - ctx, cancel := context.WithCancel(context.Background()) cancel() // force immediate hard cancel c.pgConn.Close(ctx) diff --git a/conn_test.go b/conn_test.go index d9113a75..878aa0f2 100644 --- a/conn_test.go +++ b/conn_test.go @@ -657,8 +657,8 @@ func TestFatalRxError(t *testing.T) { wg.Wait() - if conn.IsAlive() { - t.Fatal("Connection should not be live but was") + if !conn.IsClosed() { + t.Fatal("Connection should be closed") } } @@ -684,8 +684,8 @@ func TestFatalTxError(t *testing.T) { t.Fatal("Expected error but none occurred") } - if conn.IsAlive() { - t.Fatalf("Connection should not be live but was. Previous Query err: %v", err) + if !conn.IsClosed() { + t.Fatalf("Connection should be closed but isn't. Previous Query err: %v", err) } }() } diff --git a/go.mod b/go.mod index 795c7081..410f0819 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/cockroachdb/apd v1.1.0 github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f // indirect github.com/go-stack/stack v1.8.0 // indirect - github.com/jackc/pgconn v0.0.0-20190825013903-da9fc85c4404 + github.com/jackc/pgconn v0.0.0-20190825044326-6feea0c1c57d github.com/jackc/pgio v1.0.0 github.com/jackc/pgproto3/v2 v2.0.0-alpha1.0.20190609003834-432c2951c711 github.com/jackc/pgtype v0.0.0-20190824184912-ab885b375b90 @@ -28,7 +28,7 @@ require ( golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 // indirect golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a // indirect golang.org/x/text v0.3.2 // indirect - golang.org/x/tools v0.0.0-20190824210100-c2567a220953 // indirect + golang.org/x/tools v0.0.0-20190825031127-d72b05d2b1b6 // indirect golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec ) diff --git a/go.sum b/go.sum index 7e7bf0ba..ff70b4a4 100644 --- a/go.sum +++ b/go.sum @@ -25,6 +25,8 @@ github.com/jackc/pgconn v0.0.0-20190825004843-78abbdf1d7ee h1:uHUd7Cnu7QjzOqOWj6 github.com/jackc/pgconn v0.0.0-20190825004843-78abbdf1d7ee/go.mod h1:lLjNuW/+OfW9/pnVKPazfWOgNfH2aPem8YQ7ilXGvJE= github.com/jackc/pgconn v0.0.0-20190825013903-da9fc85c4404 h1:ufozZrr6aX2OernEsOAA1Ewa8agH/FdeNbWFDvespbM= github.com/jackc/pgconn v0.0.0-20190825013903-da9fc85c4404/go.mod h1:lLjNuW/+OfW9/pnVKPazfWOgNfH2aPem8YQ7ilXGvJE= +github.com/jackc/pgconn v0.0.0-20190825044326-6feea0c1c57d h1:U65ZwSZafY7s0wopPUpkess6d9cq1+sbgn29fTU8oUc= +github.com/jackc/pgconn v0.0.0-20190825044326-6feea0c1c57d/go.mod h1:lLjNuW/+OfW9/pnVKPazfWOgNfH2aPem8YQ7ilXGvJE= github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE= github.com/jackc/pgio v1.0.0/go.mod h1:oP+2QK2wFfUWgr+gxjoBH9KGBb31Eio69xUb0w5bYf8= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= @@ -129,6 +131,7 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20190425163242-31fd60d6bfdc/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190823170909-c4a336ef6a2f/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20190824210100-c2567a220953/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20190825031127-d72b05d2b1b6/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373 h1:PPwnA7z1Pjf7XYaBP9GL1VAMZmcIWyFz7QCMSIIa3Bg= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 h1:bhOzK9QyoD0ogCnFro1m2mz41+Ib0oOhfJnBp5MR4K4= diff --git a/pgxpool/conn.go b/pgxpool/conn.go index 33b6079b..ac23c5df 100644 --- a/pgxpool/conn.go +++ b/pgxpool/conn.go @@ -27,7 +27,7 @@ func (c *Conn) Release() { c.res = nil now := time.Now() - if !conn.IsAlive() || conn.PgConn().TxStatus != 'I' || (now.Sub(res.CreationTime()) > c.p.maxConnLifetime) { + if conn.IsClosed() || conn.PgConn().TxStatus != 'I' || (now.Sub(res.CreationTime()) > c.p.maxConnLifetime) { res.Destroy() return } diff --git a/stdlib/sql.go b/stdlib/sql.go index bc7766e2..df45250a 100644 --- a/stdlib/sql.go +++ b/stdlib/sql.go @@ -157,7 +157,7 @@ func (c *Conn) Prepare(query string) (driver.Stmt, error) { } func (c *Conn) PrepareContext(ctx context.Context, query string) (driver.Stmt, error) { - if !c.conn.IsAlive() { + if c.conn.IsClosed() { return nil, driver.ErrBadConn } @@ -181,7 +181,7 @@ func (c *Conn) Begin() (driver.Tx, error) { } func (c *Conn) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.Tx, error) { - if !c.conn.IsAlive() { + if c.conn.IsClosed() { return nil, driver.ErrBadConn } @@ -218,7 +218,7 @@ func (c *Conn) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.Tx, e } func (c *Conn) ExecContext(ctx context.Context, query string, argsV []driver.NamedValue) (driver.Result, error) { - if !c.conn.IsAlive() { + if c.conn.IsClosed() { return nil, driver.ErrBadConn } @@ -236,7 +236,7 @@ func (c *Conn) ExecContext(ctx context.Context, query string, argsV []driver.Nam } func (c *Conn) QueryContext(ctx context.Context, query string, argsV []driver.NamedValue) (driver.Rows, error) { - if !c.conn.IsAlive() { + if c.conn.IsClosed() { return nil, driver.ErrBadConn } @@ -257,7 +257,7 @@ func (c *Conn) QueryContext(ctx context.Context, query string, argsV []driver.Na } func (c *Conn) Ping(ctx context.Context) error { - if !c.conn.IsAlive() { + if c.conn.IsClosed() { return driver.ErrBadConn }