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

Initial sentinel dial has no timeout #68

Open
fillest opened this issue Aug 11, 2017 · 2 comments
Open

Initial sentinel dial has no timeout #68

fillest opened this issue Aug 11, 2017 · 2 comments

Comments

@fillest
Copy link

fillest commented Aug 11, 2017

sentinel.NewClientCustom uses hardcoded redis.Dial at the beginning. Shouldn't it use the same passed df DialFunc instead? So one could pass a DialTimeout wrapper.

@mediocregopher
Copy link
Owner

Hi @fillest ! thanks for submitting the issue. The df DialFunc is defined as being for the connections being made to the redis master, not the sentinel instance itself. I'd like to keep it separated as people use the DialFunc for more than just timeouts (e.g. it can be used for AUTH commands as well), so the DialFunc for the actual redis instances might not be appropriate for the sentinel.

I've had a lot of issues submitted lately about the sentinel package though, it wasn't designed particularly well when I first did so, and I'm thinking I'll add a sentinel.NewClientWithOpts function which can take in an options struct (like in cluster.NewWithOpts) and use that to address a lot of the feedback. I'll definitely keep this issue in mind when I do so, and I'll reference this issue number in the commit so you'll get a notification.

@fillest
Copy link
Author

fillest commented Aug 16, 2017

Hi.
Ah, yeah, auth..
OK, thanks. In the meantime I'm wrapping it with:

func DoWithTimeout(cb func(), t time.Duration, panicValue interface{}) {
	done := make(chan bool, 1)
	go func() {
		defer someInternalPanicReporter()
		cb()
		done <- true
	}()
	select {
	case <-done:
	case <-time.After(t):
		panic(panicValue)
	}
}

...
	var err error
	helpers.DoWithTimeout(func() {
		client, err = sentinel.NewClientCustom("tcp", addr, POOL_SIZE, dial, "smth")
	}, 3*time.Second, "Redis Sentinel communication timeout")
	if err != nil {
		panic(err)
	}

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