Proposal: refactoring rules seriesInPreviousEval

69 views
Skip to first unread message

Noam Dishon

unread,
Sep 4, 2023, 11:14:40 AM9/4/23
to Prometheus Developers

Hello,

I’m from Microsoft managed Prometheus team. We are running multiple cortex ruler instances.
I’ve noticed that a large percentage of the memory of the ruler is invested in keeping track of stale marks for series that no longer appear in the result set.

The complete set of labels of each iteration (of each rule, of each rule group) are stored in memory, in seriesInPreviousEval, until the next iteration. They are then compared to the current iteration to see if any series is no longer returns from the query. If so, that series is sent to the appender to be marked as stale.


I did a POC where the seriesInPreviousEval of each rule in stored in a disk temp file. The results shows a reduction of about 75%  in memory. There is also a reduction of around 30% in CPU. No apparent negative affect on evaluation duration.

Picture1.png
Picture2.png
Picture3.png
Picture4.png
Picture5.png

(These results are for cortex ruler, which does only rule processing, so the effect is dramatic. But I’m positive that a standalone Prometheus installation which handles a large number of rules will also benefit from different handling of seriesInPreviousEval.)

  

I would like to propose a couple of changes in this area:

2    1.  Create an interface “PreviousTimeseriesStore” (or any better name…)

2    2.      Create a default implementation MemoryPreviousTimeseriesStore. It would be similar to the current one. However, I do think we can improve the memory implementation. It currently is based on map[string]labels.Labels . The string part is made from the Bytes of the labels.

I think we can use instead a map[string]bool (effectively a hash-set).

Only if the labels are needed (which is just when a series doesn’t appear in the current result set) then we need the labels set, and it can then be reconstructed from the string. It requires FromBytes function in Labels pkg, which isn’t complicated.

C    3. Create another implementation which stores the series into the disk. The same string of each series will be stored in a file as lines. Switching to this implementation will be done via configuration.


4. Custom implementation of the interface could be passed in ManagerOptions

 

Thanks,
Noam

 

 


Bryan Boreham

unread,
Sep 5, 2023, 11:56:03 AM9/5/23
to Prometheus Developers
Are you building with -tags stringlabels?  This tends to reduce memory used by Labels.
Also I hope to reduce memory much more, though work stalled for a bit. (PR)

Since Prometheus has to store all the output series in the head, I wouldn't take Cortex ruler impact as a guide to Prometheus impact.
But if you measure that please do update us.

I expect we would accept a PR adding an interface, unless it impacted performance noticeably.

Can you share a pointer to your Cortex branch?

Bryan
(Prometheus maintainer, ex-Cortex maintainer)

Noam Dishon

unread,
Sep 26, 2023, 3:20:47 AM9/26/23
to Prometheus Developers

Thanks Bryan, 

Cortex is currently not compatible with -stringlabels . It wont compile. mainly due to Thanos not compatible. I see there's a PR to make it compatible. 

I’ve been trying to benchmark this on a Prometheus installation. However, I see that the memory is not constant, and I think it’s because tsdb holds a lot of stuff in memory. Is there a way to force it to use only disk for the sake of benchmarking?
Any way, this is my branch:
https://github.com/dishonono/prometheus/tree/stalev247
It’s not completely ready for review, but just to give you an idea.

Noam

Reply all
Reply to author
Forward
0 new messages