Skip to content
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

database/sql: Ping after statement reports good connection when it has failed #7619

Closed
gopherbot opened this issue Mar 24, 2014 · 5 comments
Closed
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 24, 2014

by zulthank:

What does 'go version' print?

go version go1.2.1 linux/amd64

What steps reproduce the problem?

1. db = sql.Open()
2. stmt = db.Prepare("INSERT INTO table VALUES($1, $2, $3, $4, $5)")
3. // now shut down the DB server
4. stmt.Exec("1", "2", "3", "4", "5")
5. err = db.Ping()

What happened?

err = nil

What should have happened instead?

err = <bad connection>

Please provide any additional information below.

- Ping before stmt.Exec() returns error, as expected
- Ping after db.Exec() (instead of stmt.Exec()) works as expected
- tested with postGreSQL DB

I could not figure out, where thee problem lies, but it seems the statement does not
update the underlying sql.db if an error occures.


Temporary solution was to define my own ping version:

func myPing(l *sql.DB) error {

    _, err := (*l).Exec("")
    if err != nil {
        return err
    }
    return nil
}
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 9, 2014

Comment 1:

Labels changed: added repo-main, release-none.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Oct 17, 2014

Comment 2 by marko@joh.to:

This appears to have been fixed in Go 1.3 via 3662d56e2402.
But it's not clear what you're trying to achieve by calling Ping() after the query in
the first place.  As far as I can tell, what nil error from Ping() means is that "there
is at least one connection to the database which we don't yet know to be bad, or a new
connection was successfully established".  I don't see where this information would be
valuable except perhaps right after sql.Open().
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Oct 20, 2014

Comment 3 by zulthank:

This can be useful, if one wants to check if the server is _still_ reachable. Probably
in a loop, since (at least at the time I have tried), I can Exec as much as I want,
without knowing if the query has actually got to the server or not, which is certainly
not assuring.
@gopherbot gopherbot added new labels Oct 20, 2014
@johto
Copy link
Contributor

@johto johto commented Dec 14, 2014

Probably
in a loop, since (at least at the time I have tried), I can Exec as much as I want,
without knowing if the query has actually got to the server or not, which is certainly
not assuring.

Now I'm truly confused. If, as you claim, you can "Exec as much as you want", how does your workaround which still uses Exec help at all? It's not obvious from your description whether you looked at the error from stmt.Exec(), but I get driver.ErrBadConn which should indicate that something's not quite right with the connectivity to the database.

This can be useful, if one wants to check if the server is still reachable.

shrug Since Ping() is fine with returning a connection from the pool without actually hitting the server, I don't consider it to be the right tool in the first place. E.g. this program:

package main

import (
    _ "github.com/lib/pq"
    "database/sql"
    "fmt"
    "time"
)

func main() {
    db, err := sql.Open("postgres", "sslmode=disable")
    if err != nil {
        panic(err)
    }

    // open maxBadConnRetries+1 connections to the DB, and keep them in the pool
    func(openConns int) {
        db.SetMaxIdleConns(openConns)
        for i := 0; i < openConns; i++ {
            rows, err := db.Query("SELECT 1")
            if err != nil {
                panic(err)
            }
            defer rows.Close()
        }
    }(11)

    stmt, err := db.Prepare("SELECT 1")
    if err != nil {
        panic(err)
    }
    fmt.Println("kill the server now")
    time.Sleep(5 * time.Second)
    _, err = stmt.Exec()
    fmt.Printf("err: %s\n", err)
    err = db.Ping()
    fmt.Printf("err: %s\n", err)
}

consistently prints:

err: driver: bad connection
err: %!s(<nil>)

which is quite bogus.

@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed release-none labels Apr 10, 2015
@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 31, 2016

Was this resolved in 4b90b7a ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.