The loops make me a bit nervous how well this would perform in a large Prometheus install that has a lot of targets.

Another option we could explore in a different PR behind feature flag:

• keep metadata in TSDB head memory per series, so that we have a map from storage.SeriesRef -> metadata
• in the queue_manager, when we receive a sample , we get the series reference as well, so we could do a single lookup in TSDB to get the metadata right then

Questions:

• how much overhead would this be?
• is there some concurrency issue? can it happen that the reference is wrong somehow by the time the queue manager get it? (reused somehow?)

(This solution could naturally evolve into persistent metadatas, but that's another story @jesusvazquez )

Hi @krajorama, thanks for suggesting. Indeed, looping through every target is very expensive. I will try to use a ref map and see if this approach is viable, maybe also writing a concurrent test for it too.

Hi @bwplotka @krajorama, I have implemented the map ref look up at 7a2e2a5. PTAL, do you think this is a good approach? If so I think I can delete ScrapeCache approach, its unit test and also change the PR desc. Also the lint is going off because I made another if else, but I will fix it when the final approach is decided

nice! First of all can we add benchmark or use existing benchmark?

We could use the build Write Request benchmark but we need to add some subcase for large the source of cache. If we use this method for existing direct mapping, maybe special case is less relevant.

But can we?

We double checked and this metadata entry in a head series[ref].memSeries is only set when metadata-wal-record is enabled (((: It's called only then https://github.com/prometheus/prometheus/blob/main/tsdb/head_append.go#L1038

And that's interesting, because we have persistent path for metadata with this flag so we are kind of going in circles. However, there might be a way for some hybrid model, where we:

• Avoid metadata-wal-records (avoid the WAL part)
• We sort of move metadata cache from scrape cache to series memSeries metadata field (with some feature flag)?

I bet we cannot "move" easily, because we need to build extra mapping from metric family name -> seriesRef/meta for old functionalities (PRW1 and metadata APIs), but maybe we can do this additionally for this RW2 lookups?

Eventually series.getByID like above (we need to somehow add this meta WITHOUT wal-records flag) is essentially sort of reusing map and potentially reusing metadata objects (if the same object as in scrape cache?), so might be worth!

So yea the biggest question is how we inject those metadata entries.

Right. So the simplest solution would be to also call Appender.UpdateMetadata if the new feature flag is enabled. We should decide on the name. But not log to WAL without the metadata in wal feature flag. This basically gives us a series level cache for metadata . On top of scrape cache for now - note that OTEL collector also depends on metadata cache at the moment. But that can also be fixed.

Yes, I like this

In our future appender interface we will allow appending meta with Append, so this will work too.

• note that OTEL collector also depends on metadata cache at the moment. But that can also be fixed.

Yes, as PRW1 and metadata APIs, so we can think later how to get those to use series level cache later on 👍🏽

It's so funny, it seems we can get more functionality (ber series mapping) near (or exactly) 0 cost, by default 🙈 just using what we have already ;P

Should we do this earlier? Do we need to build up and use memory for metadata batch in the first place?

Hm, assign is here: https://github.com/pipiland2612/prometheus/blob/80690b6aa60e9b579164af34feb3c084feba0d87/tsdb/head_append.go#L1697C2-L1697C98

So it feels it will work, just is this efficient and clean. Should we have a totally separate path for metadata-wal-records vs metadata cache update -- like a different field.

Anyway we can benchmark now and iterate.

So to benchmark this, we need to enable this config on this branch.

We can either do a hack where we enable this now in PR temporarily (default "metadata-series-cache" on) or we can adjust the benchmark as per custom benchmark flow

• https://github.com/prometheus/test-infra/blob/master/prombench/README.md#triggering-tests-via-github-comment
• https://github.com/prometheus/test-infra/tree/master/prombench/manifests/prombench#prombench-benchmark-scenario-configuration (with https://github.com/prometheus/test-infra/blob/master/prombench/manifests/prombench/benchmark/3_prometheus-test-pr_deployment.yaml changes)

Thank you for suggesting. I will solve this ASAP