vmstorage GC overhead grows on long running instance

[waldoweng]

Describe the bug

vmstorage rely on lib/workingsetcache module to cache a lots of useful data: TSIDcache, MetricNamecache, etc.
lib/workingsetcache operate mainly on two modes: split and whole mode.
lib/workingsetcache use fastcache internally, when in whole mode, internal fastcache never get reset, but do cleanup by calling fastcache.bucket.cleanLocked() function which calls delete() regularly.

func (b *bucket) cleanLocked() {
	bGen := b.gen & ((1 << genSizeBits) - 1)
	bIdx := b.idx
	bm := b.m
	for k, v := range bm {
		gen := v >> bucketSizeBits
		idx := v & ((1 << bucketSizeBits) - 1)
		if (gen+1 == bGen || gen == maxGen && bGen == 1) && idx >= bIdx || gen == bGen && idx < bIdx {
			continue
		}
		delete(bm, k)
	}
}

But this won't be enough since map of golang never shrink:

In the official standard Go runtime implementation, the backing array of a map will never shrink, even if all entries are deleted from the map. This is a form of memory wasting. But in practice, this is seldom a problem and and actually often good for program performances.
https://go101.org/optimizations/6-map.html

type bucket struct {
	mu sync.RWMutex

	// chunks is a ring buffer with encoded (k, v) pairs.
	// It consists of 64KB chunks.
	chunks [][]byte

	// m maps hash(k) to idx of (k, v) pair in chunks.
	m map[uint64]uint64

	// idx points to chunks for writing the next (k, v) pair.
	idx uint64

	// gen is the generation of chunks.
	gen uint64

	getCalls    uint64
	setCalls    uint64
	misses      uint64
	collisions  uint64
	corruptions uint64
}

memory occupied and pointer reference by fastcache.bucket.m will never go away, result in a very smooth memory leak and increase of GC overhead.

To Reproduce

1. Insert many new series data to make TSIDcache switch from split mode to whole mode
2. run for very long time (in our cases, 2 month)

Version

all cluster version

Logs

No response

Screenshots

Used command-line flags

No response

Additional information

No response

[valyala]

@waldoweng , thanks for filing the detailed bugreport!

It is unexpected that the high number of items in the bucket.m map of type map[uint64]uint64 may lead to increased GC, since this map doesn't contain pointers and Go has an optimization for such maps according to https://go101.org/optimizations/6-map.html :

If the key type and element type of a map both don't contain pointers, then in the scan phase of a GC cycle, the garbage collector will not scan the entries of the map. This could save much time.

I see the following solutions if this is the real issue (probably, it is triggered by periodic deletion of items from the map):

1. To copy periodically bucket.m items into a newly allocated map in order to eliminate allocations for superfluous items, which were already deleted:

   m := make(map[uint64]uint64, len(b.m))
   for k, v := range b.m {
      m[k] = v
   }
   b.m = m

2. To use a custom map[string]uint64 with the limited size for mapping the entry key to an offset of the entry in bucket.chunks. This implementation may use an ordinary []uint64 slice containing bucket.chunks offsets and a hash/maphash.Bytes as an efficient hash function for mapping the entry key to an index in the slice. This should reduce the number of Go objects to 1 per such a map.

[waldoweng]

@valyala, thanks for the quick response!
I should read the doc you mention again and try to find the real culprit, will report back if found.
closing this issue for now.

[valyala]

@waldoweng , I implemented the first solution mentioned above in VictoriaMetrics/fastcache@959e53c . This change has been added into VictoriaMetrics repository in the commit a7800cd . Could you try building VictoriaMetrics from this commit according to these instructions and verify whether it helps reducing GC overhead growth for long-running VictoriaMetrics instances under your workload?

If you use cluster version of VictoriaMetrics, then try building and running vmstorage from the commit 3d22f98 according to these docs.

[waldoweng]

@valyala, just want to let you know that by talking to people at golang nut, it seems to confirm this issue.
https://groups.google.com/g/golang-nuts/c/ECk8RJgdI9Q

But your implementation is coming way sooooooooooo fast!

I should try to verify this fix, will report back when I got some results, thanks!