Allow disabling of automatic Ping

[pjebs]

The gorm.Open(...) function allows feeding in a *sql.DB object instead of allowing gorm to create the object itself.

In many cases, the reason why we do it is because we already have a valid sql.DB object that we have used elsewhere. We know the connection is valid already. We don't need to waste a round trip by using .Ping() to test if it is valid.

This pull request allows us to opt-out of gorm automatically calling Ping.
The default is the current behaviour. It is also backwards-compatible so it should not affect prior code.

[pjebs]

@jinzhu Any issues with this?

[jinzhu]

Hi @pjebs

What's the problem of send another Ping?

I think it is actually good to keep that, to ensure the connection is alive.

[pjebs]

Yes I agree it is important and useful to call Ping().

...But when I create a database/sql.Db{} Object, I've already called it. I already know the connection is alive. Then when I feed it into Gorm, I increase latency a tiny bit because Gorm calls it as well.

Why not give us the option of deciding. The default behaviour of pull-request is that it already automcatically calls Ping().

[pjebs]

@jinzhu What's wrong with letting us individually decide what's best?

[pedromorgan]

as a by.stander.. where would this be in the configuration ?? is it a gorm thing, and application thing/// ?? a cron in app etc etc..

Kinda cool idea to have a ping alive sometimes.. particular on busy www etc..

So where would the config go?

[pjebs]

@pedromorgan Currently Gorm ALWAYS calls Ping() when you call Open().

Originally, gorm's Open() function ONLY created a sql.DB struct itself and managed it itself. You had no control over it other than opening it and perhaps closing it.

There are somethings in my app (and other peoples) where we needed to create/use the sql.DB object ourselves (independent of gorm). It was an optimisation if we could then feed that already-created DB object into Gorm when we occasionally wanted to use Gorm: https://github.com/jinzhu/gorm/issues/205

Someone then created a pull-request and included that functionality: https://github.com/jinzhu/gorm/pull/277

My pull-request extends that functionality so we can specify that sometimes Gorm should not call Ping() since we've already called it ourself a short moment ago before and know the connection is alive and it just wastes time (essentially equivalent to another small query).

The pull-request keeps the default behaviour (i.e. always calls Ping() ) but allows us to provide another bool value as an extra parameter that can disable automatic Ping() if we don't want it called.

It has no negatives to Gorm and only positives.

I think @jinzhu wrongly believes this Pull-request ALWAYS disables the automatic Ping().

[pjebs]

@pedromorgan Do you see any issues with this?

I think @jinzhu doesn't realise that the whole point of feeding a sql.DB object ourself is because we want to manage it ourself. If we wanted Gorm to manage it, we would have gotten Gorm to create it from the beginning.

[lunemec]

I agree there should be option to disable gorm's default Ping upon Open(). Default sql library also doesn't do this and leaves it to the app developer to decide.

[christiancadieux2]

When using postgres in a kubenetes pod, deleting the statefulset pod (but not it's service) will hang Ping(). I had to remove it from gorm.Open() so I can call my own db.Ping() that supports a timeout, otherwise Ping() takes forever to return and it's hard to tell that the database is down and it's time to switch the application to one of the postgres slaves... So that's an example where having Open() with an optional ping() is usefull so I can call my own ping(), or upgrade gorm to a ping() that supports timeout. Exampe: func PingWithTimeout(db *sql.DB, ctx context.Context) error {

errorChan := make(chan error)
pingchan := make(chan bool)

timeoutchan := make(chan bool)

go func() {
	freq := DBPingTimoutSec
	<-time.After(time.Duration(freq) * time.Second)
	timeoutchan <- true
}()

go func(db *sql.DB, e chan error, p chan bool) {
	err := db.Ping()
	if err != nil {
		e <- err
		return
	}
	p <- true

}(db, errorChan, pingchan)

select {
case <-pingchan:
	return nil

case <-timeoutchan:
	return fmt.Errorf("timeout")

case <-ctx.Done():
	return goterrs.Err(goterrs.GAPIContextCancelled)

case err := <-errorChan:
	return err
}

}