Force early cleanup of an instance when the instance list is re-sliced

[jdamick]

On Stop() & Restart() to prevent holding memory for a potentially long time, nil out the instance before re-slicing to force early cleanup.

[jdamick]

The other option would be to switch to slices.Delete(instances, i, i+1) since that will clear the removed elements.

[jdamick]

@chrisohaver / @miekg wdyt?

[jdamick]

been a couple weeks, trying someone else.. @johnbelamaric wdyt?

[jdamick]

@thevilledev can u merge? this prevents memory leakage ..

[thevilledev]

Hey @jdamick - I don't have merge permissions to the repo, but I can help with the review. I would prefer slices.Delete as it makes the intent obvious. Other than that, LGTM.

[jdamick]

@thevilledev i looked into replacing with slice but it would require changing the go.mod to bring the language support up to 1.18 for slices support. So I've left it as-is.

[thevilledev]

Yup, just realised it as well.

I did some further debugging through a memtest. This runs 10k restarts and GC + heap profile afterwards:

go test -run TestRestartDoesNotLeakInstances -v
go tool pprof -http=:8888 restart_heap.pb.gz
# or a diff of two runs
go tool pprof -top -diff_base=base.pb.gz new.pb.gz

Heap size and object counts remain stable before and after the proposed change. With the append-reslice a pointer is removed and len shortened.

@jdamick are you able to share more details about your issue, and possibly how that could be reproduced?