Reduce ingester memory by using TSDB's index #3951

bboreham · 2021-03-14T12:04:19Z

What this PR does:

RefCache is doing almost exactly the same work as TSDB, with a sharded map of series by labels.
prometheus/prometheus#8600 and prometheus/prometheus#8641 add a method to TSDB.Appender which allows us to save building a parallel cache, reducing ingester heap by about 20%.

We depend on values from GetRef() being valid to retain elsewhere.

Also added a benchmark, broadly copied from BenchmarkIngesterPush().

name              old time/op    new time/op    delta
IngesterV2Push-4     448ms ± 3%     388ms ± 1%  -13.42%  (p=0.008 n=5+5)

name              old alloc/op   new alloc/op   delta
IngesterV2Push-4    29.1MB ± 1%    27.8MB ± 0%   -4.57%  (p=0.016 n=5+4)

name              old allocs/op  new allocs/op  delta
IngesterV2Push-4     64.1k ± 0%     53.5k ± 0%  -16.52%  (p=0.008 n=5+5)

Tests show the memory reduction is around 20% when TSDB's index has been created from the WAL, in which case RefCache has a copy of all the labels for series that have received more data after restart.
Not much CPU reduction overall, since series are typically looked up many more times than they are added.

Checklist

Tests updated
NA Documentation added
CHANGELOG.md updated

pstibrany · 2021-03-15T07:39:45Z

This looks like nice improvement, but I think we should get the change into Prometheus first, to avoid using forked version.

bboreham · 2021-03-15T08:02:33Z

Yeah I thought I would get some traction here before opening the Prometheus PR.
E.g. perhaps I missed some reason for having RefCache.

bboreham · 2021-03-15T12:11:37Z

I opened the Prometheus PR; let's see what comments it gets.

pracucci

Good job @bboreham ! I like this improvement a lot. The changes as is in Prometheus would conflict with #3934: because of recent changes in TSDB appender the PR 3934 uses the refcache to also get the copy of labels to pass to the new Append() function (which merges Add() and AddFast()).

bboreham · 2021-03-20T20:14:29Z

Updated after Prometheus PR merged.

CHANGELOG.md

go.mod

pkg/ingester/ingester_v2.go

pracucci · 2021-03-23T14:25:09Z

pkg/ingester/ingester_v2.go

 		// The labels must be sorted (in our case, it's guaranteed a write request
 		// has sorted labels once hit the ingester).
-		cachedRef, copiedLabels, cachedRefExists := db.refCache.Ref(startAppend, cortexpb.FromLabelAdaptersToLabels(ts.Labels))
+
+		// Look up a reference for this series. Holding the appendLock ensures that no compaction will happen while we use it.


Holding the appendLock ensures that no compaction will happen while we use it.

Are you talking about acquireAppendLock() we have in Cortex ingester? It just ensures the forced compaction will not happen while there are on-going appends, but not the regular head compaction (see the userDB.Compact() call in compactBlocks()).

What if TSDB GetRef() would also returned interned series labels so we'll just re-pass them to the Append()? It would also allow us to reuse the same interned strings in the active series tracking.

Yes I meant what acquireAppendLock() does. Well that's a bit of a problem then.

Making GetRef() return labels would work, but shifts a bit more responsibility on to TSDB to work in a particular way.
E.g. it couldn't store the strings as a highly compressed trie.

I guess that's not a huge problem; Prometheus can just break the interface and then Cortex needs to find another solution.

I made a new version with GetRef() returning labels.

pracucci

Good job @bboreham ! Faster with less code ❤️ I just have a comment about a possible edge case.

pkg/ingester/ingester_v2.go

bboreham · 2021-03-25T18:46:00Z

I'm running this branch in production now; Go heap is 20% smaller as predicted.

So we can check the efficiency of changes to `v2Push()`. Broadly copied from `BenchmarkIngesterPush()`. Signed-off-by: Bryan Boreham <[email protected]>

prometheus/prometheus#8600 adds a method to `TSDB.Appender` which allows us to save building a parallel cache, reducing ingester heap by about 20%. We depend on values from GetRef() remaining valid while v2Push() uses them. Currently the only way a ref can be invalidated is by a head compaction, which cannot happen while v2Push() holds the append lock. Signed-off-by: Bryan Boreham <[email protected]>

Now we only need to make a copy if GetRef() returns zero Note Prometheus update brings in JSON marshalling of model.Duration prometheus/common#280 Signed-off-by: Bryan Boreham <[email protected]>

Signed-off-by: Bryan Boreham <[email protected]>

Pinned gRPC and other dependencies changed by the update in Prometheus to avoid taking so much change on this PR. Signed-off-by: Bryan Boreham <[email protected]>

bboreham · 2021-03-30T09:58:44Z

I've now pinned all the changed dependencies except Prometheus in this PR,
and created a new PR which has the 18,000 lines of dependency updates #4028

pracucci

Solid work @bboreham! Sorry for my late reply 👏

pull-request-size bot added the size/XL label Mar 14, 2021

bboreham mentioned this pull request Mar 15, 2021

[RFC] Add method to get reference number for TSDB Appender prometheus/prometheus#8600

Merged

pracucci reviewed Mar 15, 2021

View reviewed changes

bboreham mentioned this pull request Mar 15, 2021

[RFC] Combine Appender.Add and AddFast into a single Append method. prometheus/prometheus#8489

Merged

bboreham force-pushed the simplify-v2push branch 2 times, most recently from ed0a59e to 569eeaf Compare March 20, 2021 20:00

bboreham force-pushed the simplify-v2push branch 3 times, most recently from e5cd7ce to a4a468a Compare March 22, 2021 14:00

pracucci reviewed Mar 23, 2021

View reviewed changes

bboreham force-pushed the simplify-v2push branch from 0377e54 to 130b2b7 Compare March 24, 2021 09:01

bboreham mentioned this pull request Mar 24, 2021

Make GetRef() return the label set prometheus/prometheus#8641

Merged

bboreham force-pushed the simplify-v2push branch from 48570c3 to bef9df0 Compare March 24, 2021 16:07

pull-request-size bot added size/XXL and removed size/XL labels Mar 24, 2021

pracucci approved these changes Mar 25, 2021

View reviewed changes

pkg/ingester/ingester_v2.go Show resolved Hide resolved

pkg/ingester/ingester_v2.go Show resolved Hide resolved

bboreham force-pushed the simplify-v2push branch 2 times, most recently from 52a7946 to bc9e4fa Compare March 26, 2021 11:08

bboreham added 5 commits March 30, 2021 09:22

Add BenchmarkIngesterV2Push

13b531a

So we can check the efficiency of changes to `v2Push()`. Broadly copied from `BenchmarkIngesterPush()`. Signed-off-by: Bryan Boreham <[email protected]>

New version of GetRef() that returns labels

4ee2538

Now we only need to make a copy if GetRef() returns zero Note Prometheus update brings in JSON marshalling of model.Duration prometheus/common#280 Signed-off-by: Bryan Boreham <[email protected]>

Add comment on use of copiedLabels

39c9801

Signed-off-by: Bryan Boreham <[email protected]>

Update to Prometheus main branch

29c20d2

Pinned gRPC and other dependencies changed by the update in Prometheus to avoid taking so much change on this PR. Signed-off-by: Bryan Boreham <[email protected]>

bboreham force-pushed the simplify-v2push branch from bc9e4fa to 29c20d2 Compare March 30, 2021 09:44

pull-request-size bot removed the size/XXL label Mar 30, 2021

pull-request-size bot added the size/XL label Mar 30, 2021

bboreham mentioned this pull request Mar 30, 2021

Update Prometheus to commit c7a62b95cea0 #4028

Closed

1 task

pracucci approved these changes Mar 31, 2021

View reviewed changes

pracucci merged commit ad3ea42 into master Mar 31, 2021

pracucci deleted the simplify-v2push branch March 31, 2021 07:40

bboreham mentioned this pull request Apr 12, 2021

Proposal: make ref cache labels share storage with TSDB #3817

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce ingester memory by using TSDB's index #3951

Reduce ingester memory by using TSDB's index #3951

bboreham commented Mar 14, 2021 •

edited

Loading

pstibrany commented Mar 15, 2021

bboreham commented Mar 15, 2021

bboreham commented Mar 15, 2021

pracucci left a comment

bboreham commented Mar 20, 2021

pracucci Mar 23, 2021

pracucci Mar 23, 2021

bboreham Mar 23, 2021

bboreham Mar 23, 2021

pracucci left a comment

bboreham commented Mar 25, 2021

bboreham commented Mar 30, 2021

pracucci left a comment

Reduce ingester memory by using TSDB's index #3951

Reduce ingester memory by using TSDB's index #3951

Conversation

bboreham commented Mar 14, 2021 • edited Loading

pstibrany commented Mar 15, 2021

bboreham commented Mar 15, 2021

bboreham commented Mar 15, 2021

pracucci left a comment

Choose a reason for hiding this comment

bboreham commented Mar 20, 2021

pracucci Mar 23, 2021

Choose a reason for hiding this comment

pracucci Mar 23, 2021

Choose a reason for hiding this comment

bboreham Mar 23, 2021

Choose a reason for hiding this comment

bboreham Mar 23, 2021

Choose a reason for hiding this comment

pracucci left a comment

Choose a reason for hiding this comment

bboreham commented Mar 25, 2021

bboreham commented Mar 30, 2021

pracucci left a comment

Choose a reason for hiding this comment

bboreham commented Mar 14, 2021 •

edited

Loading