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

All the requests in admin.go don't specify a timeout and none is allowed to be set causing them to ErrRequestTimedOut #1120

Closed
gadkrumholz opened this issue Jun 25, 2018 · 6 comments

Comments

@gadkrumholz
Copy link

gadkrumholz commented Jun 25, 2018

Versions

Sarama Version: master@46cf3e2cf1acef7876068f66cf69ec31aad2d0b2
Kafka Version: 1.10
Go Version: 1.10

Configuration

N/A

What configuration values are you using for Sarama and Kafka?
N/A

Logs

N/A

Problem Description

When a long-running operation like creating a topic with a lot of partitions and replicasets or when changing a topic's # of partitions (the 2 examples we've run into this with) you may sometimes get ErrRequestTimedOut even though the operation completes successfully.

Could you please expose the Timeout parameter for the requests to be passed in with each method call or have it use the same Timeout parameter from the config passed into NewClusterAdmin(...)?

@eapache
Copy link
Contributor

eapache commented Jun 26, 2018

I don't think there's an existing config value which makes sense for those timeouts, but I'm happy to take a PR adding one.

@andy2046
Copy link
Contributor

andy2046 commented Jul 28, 2018

@eapache do you think adding Cluster Timeout into existing Config is the right way to solve this? or you have better idea, I'd like to raise a PR for this

type Config struct {
	Cluster struct {
		Timeout time.Duration
	}
}

@eapache
Copy link
Contributor

eapache commented Jul 28, 2018

Something like that. I could see an argument for putting this value under Metadata, or for creating a new Admin struct in config. The name Cluster on its own isn't specific enough for me.

@andy2046
Copy link
Contributor

@eapache Thanks for the advise, I raised a PR 1142 with new Admin struct. please help to review and comment, thanks.

@andy2046
Copy link
Contributor

@eapache should we close this issue?

@d1egoaz
Copy link
Contributor

d1egoaz commented Aug 22, 2019

fixed by #1142
thanks

@d1egoaz d1egoaz closed this as completed Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants