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

Make concurrent fetch of samples #11

Open
fogfish opened this issue Apr 5, 2024 · 2 comments · May be fixed by #12
Open

Make concurrent fetch of samples #11

fogfish opened this issue Apr 5, 2024 · 2 comments · May be fixed by #12
Labels
good first issue Good for newcomers

Comments

@fogfish
Copy link
Member

fogfish commented Apr 5, 2024

The utility fetches samples sequentially, for large jobs it might take seconds. It would be beneficial to run it in parallel.

https://github.com/zalando/rds-health/blob/main/internal/insight/insight.go#L68

@fogfish fogfish added the good first issue Good for newcomers label Apr 5, 2024
@rafiramadhana
Copy link

rafiramadhana commented Apr 5, 2024

@fogfish if we have >=2 concurrent in.fetch calls and some of them are returning errors, which error that we should return?

does ordering matter? or we can just return any of the errors (then prevent further concurrent in.fetch calls in that request scope)

@fogfish
Copy link
Member Author

fogfish commented Apr 6, 2024

@rafiramadhana Thank you for raising the interest.

Exiting loop fails if any of I/O with AWS Performance Insight fails. I've not found a good way to achieve graceful degradation because all fetched metrics are used later by the "rules" engine.

	samples := map[string]Samples{}
	for _, chunk := range chunks {
		set, err := in.fetch(ctx, dbiResourceId, dur, chunk...)
		if err != nil {
			return nil, err
		}

		for k, v := range set {
			samples[k] = v
		}
	}

I would not change the behaviour of the function, I would fail also if any of in.fetch request fails. The first error would be sufficient to return.

The ordering does not matter. Please not the results of in.fetch are aggregated into map. The rule engine uses metrics name and fetched from the map (in Golang oder of keys is non deterministic in the map).

@AnshulMalik AnshulMalik linked a pull request Apr 7, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants