Hi Brian, Thanks for your input !
> A large part of the performance gains of Prometheus 2.x is attributed to > it not using maps, so I would be extremely cautious about changing it for > the sake of SD which is not the hot path. n is generally small for > ingestion, so complexity alone can be misleading. > Indeed, I was wrong. I tried to use a map in labels.Builder and we moved from 7 CPUs to 12 CPUs used instantly. So I revert this change in the PR immediately. But still in the particular case of the SD, using a map in the method buildTargetLabels <https://github.com/prometheus/prometheus/pull/10662/files#diff-f9baaa77ffadc6027a5881865a568d273ceac36a04528b6e6cb7a745bf95eac3R508> helps to gain around 1 to 2 CPUs on 9. (I tried first with labels.Builder). At the end, having a logic to not recompute the totality of the targets but just a piece of that doesn't seem to work enough to see a huge drop of the CPU usage. The PR I opened can still be merged even if the diff is not that important. A combination of this PR and 10634 <https://github.com/prometheus/prometheus/pull/10634> can help to mitigate the issue. But I'm afraid the only real fix is to have another channel in the SD that will be used to receive a unique update of a target. In the particular case of the k8s SD it will be used to FW the update of the PODs (for example). Augustin. Le jeu. 12 mai 2022 à 10:05, Brian Brazil <[email protected]> a écrit : > On Wed, 11 May 2022 at 10:06, Augustin Husson <[email protected]> > wrote: > >> Dear Prometheus developers, >> >> I'm currently working on trying to optimize the synchronization of the >> targets in the Prometheus Discovery. (Issue is here >> <https://github.com/prometheus/prometheus/issues/8014>) >> >> And after a couple of investigations and tests on a real environment, we >> figured out that actually the usage of the method Set of the >> labels.Builder >> <https://github.com/prometheus/prometheus/blob/main/model/labels/labels.go#L388> >> cost a lot, and more generally the global usage of labels.Builder cost. >> >> In this PR <https://github.com/prometheus/prometheus/pull/10662> that is >> supposed to fix the issue mentioned above, I started to replace the usage >> of labels.Builder by a map and it immediately optimized the code. The >> reason is because it reduces the complexity of the different methods. Like >> for the method Set, we moved from o(n2) to o(n). >> >> So my question is: Would it be ok to change the internal implementation >> of labels.Builder by using a map instead of multiple different slices ? It >> won't change the API exposed, just the internal implementation. >> > > A large part of the performance gains of Prometheus 2.x is attributed to > it not using maps, so I would be extremely cautious about changing it for > the sake of SD which is not the hot path. n is generally small for > ingestion, so complexity alone can be misleading. > > When I last looked at the SD code the main performance issue looked to be > that it had not been converted to using labels.Builder yet, and the > continuous conversions from a map was what the main problem looked to be. > Along with other low hanging fruit. > > I believe it will help to optimize smoothly (for example) the method >> relabel.Process >> <https://github.com/prometheus/prometheus/blob/main/model/relabel/relabel.go#L193> >> > > This isn't on a hot path these days, as it's only run the first time we > see a given sample. > > Brian > > >> >> Cheers, >> Augustin. >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Prometheus Developers" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/prometheus-developers/CAOJizGe0AxLGw5G9BM1Z6Menamq%3DxTMHzFXjj73fTqM5%2BZgz5A%40mail.gmail.com >> <https://groups.google.com/d/msgid/prometheus-developers/CAOJizGe0AxLGw5G9BM1Z6Menamq%3DxTMHzFXjj73fTqM5%2BZgz5A%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > > > -- > Brian Brazil > www.robustperception.io > -- You received this message because you are subscribed to the Google Groups "Prometheus Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAOJizGeH2HQ0PFsxHsku3v%2B0r6tGG7kwiDCRMVB88mysiHzgbg%40mail.gmail.com.
