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 <brian.bra...@robustperception.io> a écrit : > On Wed, 11 May 2022 at 10:06, Augustin Husson <husson.augus...@gmail.com> > 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 prometheus-developers+unsubscr...@googlegroups.com. >> 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 prometheus-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAOJizGeH2HQ0PFsxHsku3v%2B0r6tGG7kwiDCRMVB88mysiHzgbg%40mail.gmail.com.
