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

MustPing with timeout #58

Open
heynemann opened this issue Feb 15, 2017 · 3 comments
Open

MustPing with timeout #58

heynemann opened this issue Feb 15, 2017 · 3 comments

Comments

@heynemann
Copy link

Hey @mgutz do you think you could make MustPing receive an optional parameter (with ...) to specify a timeout?

The issue I'm trying to avoid is that upon launching my servers, if there's no connectivity to a database, they'll just stay there trying indefinitely to connect, silently. The behavior I would expect of a well-behaved server is to fail with the proper error ("Could not connect to database at somethingsomething:3845" or something like that). That would trigger our error handling infrastructure and we'd be notified.

Do you think this is something you'd like to support? I can contribute with a PR if you'd like.

@mgutz
Copy link
Owner

mgutz commented Feb 15, 2017

The convention in Go is anything prefixed by "Must" will panic. If you'd like to submit a PR for a ShouldPing function, I can add that.

@heynemann
Copy link
Author

I totally agree, the problem here is not that it won't panic (although I will send a PR with ShouldPing as I'd rather have that). The problem here is that it will never panic. It will just keep retrying time after time to ping.

I looked at the backoff lib and it has a MaxElapsedTime property that can be used for this purpose, like:

func shouldPing(db *sql.DB, timeout time.Duration) error {
	var err error
	b := backoff.NewExponentialBackOff()
	b.MaxElapsedTime = timeout
	ticker := backoff.NewTicker(b)

	// Ticks will continue to arrive when the previous operation is still running,
	// so operations that take a while to fail could run in quick succession.
	for range ticker.C {
		if err = db.Ping(); err != nil {
			continue
		}

		ticker.Stop()
		return nil
	}

	return fmt.Errorf("Could not ping database!")
}

I'm doing a PR now! :)

@heynemann
Copy link
Author

Created PR #59.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants