Receive: poor memory efficiency when (un)marshaling data

[fpetkovski]

Is your proposal related to a problem?

I took a profile of a bloated Thanos Receiver, and noticed that a huge amount of memory is being allocated for just marshaling and unmarshaling data. This is a big surprise as I would expect the majority of the memory usage would come from TSDBs.

Describe the solution you'd like

We should investigate why marshaling data has such a significant impact on memory.

Describe alternatives you've considered

N/A

Additional context

Here is a link to the profile: https://pprof.me/ccc9a28/

[bwplotka]

Agree, thanks! 💪🏽 We see similar on our side I think @matej-g @philipgough

[douglascamata]

@fpetkovski: Why would TSDBs allocate more memory than marshalling? Aren't TSDBs just pointing to memory previously allocated by the marshalling? 🤔

[fpetkovski]

TSDBs hold memory for a longer time period, and I assume we don't marshal everything from them during series requests.

[matej-g]

I think we've seen similar numbers in our profiles, for us the biggest issue was switch to snappy compression on 0.28.0, as I was mentioning in #5575 (comment), which after we switched back to no compression we got back to numbers similar to 0.27.0. @philipgough has now run a couple more tests comparing with / without compression and the issue seems to be especially pronounced on instances where we ingest big requests. @douglascamata @philipgough feel free to add more from your observations.

But speaking solely on (un)marshalling, yes, that could be the next thing to look at. Although I wonder where / what to start with 🤔.

[douglascamata]

One of the question I have in mind at the moment is: is marshalling's high in-use memory numbers due to few huge gRPC messages? Does it get better in the scenario with more numerous but smaller messages?

[matej-g]

Another related issue, I'm wondering what kind of improvement we'd see if / when we move to the Vitess framework #4557

[fpetkovski]

I was going to suggest the same, we might be able to start pooling requests once that is merged.

[fpetkovski]

Could be related to klauspost/compress#442

[juanrh]

Hi,

I'm also seeing this in a prod cluster where in the memory profile just before the OOM the top for -sample_index=inuse_space has 97% of the memory allocated for:

google.golang.org/grpc.(*parser).recvMsg
github.com/thanos-io/thanos/pkg/store/storepb.(*Chunk).Unmarshal
github.com/thanos-io/thanos/pkg/store/storepb.(*Series).Unmarshal
github.com/thanos-io/thanos/pkg/query.removeExactDuplicates
github.com/thanos-io/thanos/pkg/store/storepb.(*AggrChunk).Unmarshal

so most of that is gRPC serialization, but query.removeExactDuplicates is also allocating 1.94GB (flat 12.53%) for ret := make([]storepb.AggrChunk, 0, len(chks)). For what I see removeExactDuplicates is only called as s.currChunks = removeExactDuplicates(s.currChunks) in func (s *promSeriesSet) Next() bool, so a complementary optimization could be removing duplicates it in place, for example as follows:

$ git diff
diff --git a/pkg/query/iter.go b/pkg/query/iter.go
index 6e9e051fe..2b49e6174 100644
--- a/pkg/query/iter.go
+++ b/pkg/query/iter.go
@@ -77,17 +77,14 @@ func removeExactDuplicates(chks []storepb.AggrChunk) []storepb.AggrChunk {
        if len(chks) <= 1 {
                return chks
        }
-
-       ret := make([]storepb.AggrChunk, 0, len(chks))
-       ret = append(ret, chks[0])
-
+       i := 0
        for _, c := range chks[1:] {
-               if ret[len(ret)-1].Compare(c) == 0 {
-                       continue
+               if chks[i].Compare(c) != 0 {
+                       i++
+                       chks[i] = c
                }
-               ret = append(ret, c)
        }
-       return ret
+       return chks[:i+1]
 }
 
 func (s *promSeriesSet) At() storage.Series {

What do you think?

[matej-g]

@juanrh definitely! Could you open (even if a draft) PR with the proposed changes? We could discuss it in full there and perhaps look at some benchmarks.

[juanrh]

@matej-g I just sent #5795 for that