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

feature request - allow to specify client go qps and burst #16

Closed
grzesuav opened this issue Aug 22, 2022 · 12 comments
Closed

feature request - allow to specify client go qps and burst #16

grzesuav opened this issue Aug 22, 2022 · 12 comments

Comments

@grzesuav
Copy link

I see throttling messages

Waited for 1.197551744s due to client-side throttling,

would be good to be able to tune that setting

@leovct
Copy link

leovct commented Aug 29, 2022

Without this feature, the latest versions of the kubernetes-exporter are not suitable for us, almost no events are reported in ElasticSearch...

@rfvmonteiro
Copy link

The original repo already has an open pull request to this feature. Maybe the owner can add it here.
opsgenie#171

It is really useful to monitor larger cluster with a high number of events.

@mustafaakin
Copy link

Hi, I can manually checkout that code to merge, but pinging @lobshunt just in case. lobshunt
If no response, I can merge it later today or tomorrow if it's OK?

@mustafaakin
Copy link

Seems like I cannot ping arbitrary people from other repository. So I'm merging this and some other PRs there manually today. Stay tuned.

@mustafaakin
Copy link

mustafaakin commented Aug 31, 2022

I'm trying to come up with sensible defaults, any opinions? Defaults in the rest/config.go is as follows:

// QPS indicates the maximum QPS to the master from this client.
// If it's zero, the created RESTClient will use DefaultQPS: 5
QPS float32

// Maximum burst for throttle.
// If it's zero, the created RESTClient will use DefaultBurst: 10.
Burst int

@rfvmonteiro
Copy link

These kind of configs are the ones you should tune for your context. For new and small clusters the defaults DefaultQPS:5 and DefaultBurst:10 are enough. Its hard for you to set a sensible default because you just don't know in which context this tool will be used.
In your place I keep it as the original defaults.

@mustafaakin
Copy link

Makes sense, I'll note it in README clearly to point out you need to set them properly for large clusters.

@sidewinder12s
Copy link

I'd wondered if this would solve the original issue reported in opsgenie#159

I've seen the same, after some time the exporter just stops dumping events to sinks, usually in high traffic clusters. Restarting gets it dumping again.

@diogenxs
Copy link

Hi y'all, I applied the solution proposed on opsgenie#171 to my fork and it fixed the problem for me.
I wonder if we can apply the same here since I don't want to keep a fork for it 😬
I can do the PR if needed.

@mustafaakin
Copy link

Sorry for the long release delay. I merged the changes and the release will be available soon!

@lobshunter
Copy link

Hi, I can manually checkout that code to merge, but pinging @lobshunt just in case. lobshunt If no response, I can merge it later today or tomorrow if it's OK?

Just found this new active fork. I am glad to see this fix finally got merged. 🎉

@xmcqueen
Copy link

xmcqueen commented Nov 4, 2022

I was going to add this too! I'm glad to see this.

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

8 participants