memory leak in mockstore

[GMHDBJD]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

package main

import (
	"os"
	"runtime/pprof"

	"github.com/pingcap/tidb/session"
	. "github.com/pingcap/tidb/store/mockstore"
)

func testMock() {
	opts := []MockTiKVStoreOption{WithStoreType(MockTiKV)}
	store, _ := NewMockStore(opts...)
	defer store.Close()

	dom, _ := session.BootstrapSession(store)
	defer dom.Close()
}

func main() {
	for i := 0; i < 10; i++ {
		testMock()
	}
	f, _ := os.Create("heap_profile")
	pprof.WriteHeapProfile(f)
}

2. What did you expect to see? (Required)

free the memory when dom.Close()

3. What did you see instead (Required)

4. Affected version (Required)

tidb master

5. Root Cause Analysis

[g1eny0ung]

/label component/store

[tiancaiamao]

Yes, I can confirm the memory leak.
It's caused by some global variable references, but I do not know which global variable.

[tiancaiamao]

Those are the global variables of the test program.
I use dlv attach to get them.

global.LOG

[csuzhangxc]

Most of global variables will not increase memory usage, I think we can just let them stay there.

But some of them may increase memory usage after repeated NewMockStore/BootstrapSession/Close, I think we should fix them.

[tiancaiamao]

The global variable itself does not increase memory usage, but the may reference some other variable and make the variable can not be released by GC, thus leak.

[tiancaiamao]

I find one place of the leak

tidb/expression/builtin_vectorized.go

Line 48 in c9c0daf

var globalColumnAllocator = newLocalSliceBuffer(1024)

@qw4990

[csuzhangxc]

@tiancaiamao @qw4990 Is anyone still working on this?

[tiancaiamao]

No progress on this issue. @csuzhangxc
I've tried my best but found it's too difficult to check all the global variable references.

[csuzhangxc]

@tiancaiamao can you fix the one you found above first?

tidb/expression/builtin_vectorized.go

Line 48 in c9c0daf

var globalColumnAllocator = newLocalSliceBuffer(1024)

[csuzhangxc]

ping anyone

[qw4990]

Sorry for my late reply. Could you please give me your go.mod? I cannot reproduce this problem now. @csuzhangxc

[qw4990]

Since this problem only occurs in mockstore, its severity is degraded to minor.

[GMHDBJD]

test.tar.gz
I reproduce with master tidb.(both mocktikv and unistore)

[zhangysh1995]

/pick-up

[ti-challenge-bot]

It is not a pickable issue!

Details

Tip : If you want this issue to be picked, you need to add a challenge-program label to it.

Warning: None

[zhangysh1995]

@ichn-hu Could you please add a label for the bug fixing challenge that I could pick up this issue?

[wjhuang2016]

@ichn-hu Could you please add a label for the bug fixing challenge that I could pick up this issue?

You can fire a PR to fix it directly.

[GMHDBJD]

Ping

[lance6716]

found that domap still holds a reference to Domain after Domain is closed (the key is UUID), but after cleaning the map the memory still can't decrease

tidb/session/tidb.go

Lines 103 to 105 in 7755d25

	domap = &domainMap{
	domains: map[string]*domain.Domain{},
	}

[sleepymole]

I found statisticsList will always hold the statistics which is probably a *ddl.ddl.

tidb/sessionctx/variable/statusvar.go

Lines 46 to 50 in c26038b

	func RegisterStatistics(s Statistics) {
	statisticsListLock.Lock()
	statisticsList = append(statisticsList, s)
	statisticsListLock.Unlock()
	}

After clear domap and comment the follow two lines, the memory won't increase all the time.

tidb/bindinfo/handle.go

Line 120 in c26038b

variable.RegisterStatistics(handle)

tidb/ddl/ddl.go

Line 386 in c26038b

variable.RegisterStatistics(d)

[github-actions]

Please check whether the issue should be labeled with 'affects-x.y' or 'backport-x.y.z',
and then remove 'needs-more-info' label.