Proposal for memory limiting in the scrape loop

[dashpole]

As @bwplotka presented in his "Scrape Trolley Dillema" talk last year at promcon, Prometheus could use a mechanism to prevent OOMs caused by short-term memory usage of scraping targets. This has also been a request from some users of the OTel collector's Prometheus receiver.

PoC: prometheus/prometheus@main...dashpole:prometheus:memory_limiter_simple

cc @bernot-dev @ArthurSens

[bwplotka]

FYI: Talk link: https://youtu.be/ulHQUCarjjo?list=PLoz-W_CUquUlHOg314_YttjHL0iGTdE3O

[bwplotka]

IMO this is a great plan, something I'd like to see 👍🏽

[bwplotka]

I'm supportive. It should break the ice on the non trivial issues you listed and it's elegant. This would also help with Otel-collector Prometheus scrapes.

• prometheus/prometheus#17109
• prometheus/prometheus#13939
• prometheus/prometheus#11306
• prometheus/prometheus#16917

Disclaimer @dashpole works with me at Google, so I'm not going to merge until we have buy-in from other maintainers.

WDYT @krajorama @roidelapluie @bboreham @ArthurSens ?

[saswatamcode]

Would love to see this!

[dashpole]

Note to reviewers: The name can be debated/changed during review.

[dashpole]

Discussed this at the Prometheus Dev-summit today. Feedback:

• @SuperQ has a lot of ideas around this, and we should discuss further.
• @bboreham says there is something similar in Mimir (but is AGPL), so this general idea has some backing by production usage.
• Memory usage is cyclic, and tends to be highest right before compaction.
• Series already in the index don't add to long-term memory usage. It is new series that grow the long-term memory usage.

[SuperQ]

Another memory mitigation that could be done would be to pause rule evaluations. Specifically recordings could be paused since they run queries which can be an alloc churn impact.

[dashpole]

@SuperQ Do you think it should just pause rule evaluations, or should it cause all PromQL queries to fail? If the issue is queries, seems like pausing all queries would be better? OTOH, intermittently not being able to query prometheus seems like it would make it hard to tell what is going wrong.

[SuperQ]

That's a good question. Pausing rules is nice because you can, in theory, resume without loss.

Pausing all PromQL is also a good option.

For "hard to tell what's going on" is why meta-monitoring exists. Having a separate Prometheus-to-watch-Prometheus is usually recommended for that. 😁

[yeya24]

I like this proposal and we saw quite a lot of OOM kills of scraping due to quick churning during deployment or any sudden load increase.

One thing I am wondering is that if there is a way to deal with scraper OOM kills without losing data. That's definitely more for future but if the scraper can scrape the metric snapshot with a timestamp then the missing metrics can be backfilled with OOO

[SuperQ]

It would be interesting to have some heuristic that uses scrape_series_added, or maybe something like this:

increase(prometheus_tsdb_head_series_created_total[5m)
/
prometheus_tsdb_head_series

Canceling scrapes is, IMO, an action of last resort because it means we're dropping data permanently.

[dashpole]

I've had some time to think about how to incorporate recording rules and queries. Taking a broader look at the entire prometheus server, I think we could apply memory limiting in some form to:

• scrape_configs (this proposal)
• otlp: Reject incoming OTLP metrics
• remote_read: Reject incoming remote_read
• rule_files: Pause recording rules (I assume we don't want to pause alerting rules!?)
• compaction: Pause TSDB compaction.
• Parts of the API (queries, targets, series, metadata)?

I think the best way to address this is to make the feature naming more generic ("memory_limiter" instead of "scrape_memory_limiter") and to bundle memory limiting behavior behind a "hard" limit (data loss), and a "soft" limit (no data loss). That seems like it would be easy to understand, comprehensive, and configurable enough for most usage.

When the "soft" limit is met:

• Pause compaction
• Pause recording rules
• Reject PromQL queries (and other API queries?)

When the "hard" limit is met:

• Fail scrapes
• Reject OTLP
• Reject remote read

Question for reviewers:

Would you prefer I:

1. Expand the scope of the proposal to include all of the above behavior?
2. Just change the name of "scrape_memory_limiter" to "memory_limiter", and leave the rest of the topics (soft limits, recording rules, compaction, etc) as future enhancements?
3. Expand the scope of the proposal to only include pausing recording rules (in addition to failing scrapes).

[SuperQ]

I think expanding the proposal to support multiple mitigation options is a good idea.

Since all of the options discussed so far have pretty impactful downsides different users are going to want opt-in to different options.

We likely want to support configuring different options, have some kind of tunable heuristics, etc.

But all of it is going to need to be tested for various failure modes.

[dashpole]

@SuperQ I expanded the proposal. The things I want to make sure we get right during the proposal stage are:

• Categories: Did I miss any important sources of short-term memory usage? I've initially excluded standard promql queries, or other debugging parts of the API surface (e.g. targets or metadata pages).
• Soft and hard limits: Does this categorization of mitigations into "destructive" (hard) vs "non-destructive" (soft) make sense? Or should we allow setting more granular memory thresholds for each mitigation?
• Configuration: Is turning on/off each mitigation enough configurability? Or will we need something more granular at the global level?