Skip to content

scrape/scrape_test.go: reduce the time it takes to reload the manager#14447

Merged
beorn7 merged 3 commits intoprometheus:mainfrom
krajorama:faster-TestNativeHistogramMaxSchemaSet
Sep 26, 2024
Merged

scrape/scrape_test.go: reduce the time it takes to reload the manager#14447
beorn7 merged 3 commits intoprometheus:mainfrom
krajorama:faster-TestNativeHistogramMaxSchemaSet

Conversation

@krajorama
Copy link
Member

TestNativeHistogramMaxSchemaSet took over 3x5s to complete because there's a minimum reload interval.

I've made the testcases run in parallel and reduced the reload interval to 10ms. Now the test runs in around 0.1-0.2 seconds.

Ran test 10000 times to check if it's flaky.

TestNativeHistogramMaxSchemaSet took over 3x5s to complete because
there's a minimum reload interval.

I've made the testcases run in parallel and reduced the reload interval
to 10ms. Now the test runs in around 0.1-0.2 seconds.

Ran test 10000 times to check if it's flaky.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@krajorama krajorama requested a review from bboreham July 9, 2024 17:58
func (m *Manager) reloader() {
reloadIntervalDuration := m.opts.DiscoveryReloadInterval
if reloadIntervalDuration < model.Duration(5*time.Second) {
if !m.opts.skipDiscoveryReloadIntervalCheck && reloadIntervalDuration < model.Duration(5*time.Second) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a flag, I'm suggesting to remove this hidden min value https://github.com/prometheus/prometheus/pull/13147/files#r1404481390

@beorn7
Copy link
Member

beorn7 commented Aug 29, 2024

#13147 is merged now. So this could be updated accordingly, and then we can ship it?

@beorn7
Copy link
Member

beorn7 commented Sep 25, 2024

@krajorama should be easy to rebase, are you still up to the task?

…stogramMaxSchemaSet

# Conflicts:
#	scrape/manager.go
#	scrape/scrape_test.go
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Copy link
Member

@beorn7 beorn7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

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

Successfully merging this pull request may close these issues.

3 participants