zalando / rds-health

discover anomalies, performance issues and optimization within AWS RDS
MIT License
26 stars 4 forks source link

Make concurrent fetch of samples #11

Open fogfish opened 6 months ago

fogfish commented 6 months ago

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

rafiramadhana commented 6 months ago

@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 commented 6 months ago

@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).