From ff9f7744657e3f5529d1af9cf234a2b4aef61506 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Wed, 4 Dec 2019 15:54:48 -0500 Subject: [PATCH 1/2] storage: don't let cmdQ buffers grow without bound This commit places a limit on the size of the overlaps buffer than the CommandQueue uses to avoid repeat allocations in getOverlaps. It's risky to allow this buffer to grow without bounds and never be reclaimed. Release note (bug fix): The CommandQueue no longer holds on to buffers if they grow to large. This prevents unbounded growth of memory that may never be reclaimed. --- pkg/storage/command_queue.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/storage/command_queue.go b/pkg/storage/command_queue.go index d2b54610d223..4beca2b4b564 100644 --- a/pkg/storage/command_queue.go +++ b/pkg/storage/command_queue.go @@ -629,7 +629,13 @@ func (cq *CommandQueue) getOverlaps( // Both reads and writes must wait on other writes, depending on timestamps. cq.writes.DoMatching(cq.collectOverlappingWritesRef, rng) overlaps := cq.overlaps - cq.overlaps = cq.overlaps[:0] + if cap(cq.overlaps) > 1024 { + // Limit the maximum size that the overlaps buffer can grow to. We don't + // want to hold on to a potentially unbounded size slice. + cq.overlaps = nil + } else { + cq.overlaps = cq.overlaps[:0] + } return overlaps } From 315781d37db553fe18f66fada6b52b314695b473 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Wed, 4 Dec 2019 15:55:42 -0500 Subject: [PATCH 2/2] storage: clear *cmd references in cmdQ buffers before re-use This commit ensures that the *cmd references in the overlaps buffer that the CommandQueue uses to avoid repeat allocations are cleared after use. This prevents the buffer from accidentally holding references to *cmd objects and preventing them from being GCed. Release note (bug fix): The CommandQueue makes sure to clear references to objects in its buffers to allow those objects to be reclaimed by the garbage collector. --- pkg/storage/command_queue.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/storage/command_queue.go b/pkg/storage/command_queue.go index 4beca2b4b564..74ecb230e5b5 100644 --- a/pkg/storage/command_queue.go +++ b/pkg/storage/command_queue.go @@ -436,6 +436,10 @@ func (cq *CommandQueue) getPrereqs( restart = cq.expand(c, true /* isInserted */) || restart } if restart { + // Clear all *cmd references in overlaps, per the getOverlaps contract. + for j := range overlaps { + overlaps[j] = nil + } i-- continue } @@ -614,7 +618,9 @@ func (cq *CommandQueue) getPrereqs( } // getOverlaps returns a slice of values which overlap the specified -// interval. The slice is only valid until the next call to getOverlaps. +// interval. The slice is only valid until the next call to getOverlaps +// and all elements should be nil-ed out to avoid holding references to +// *cmd objects and preventing GC. func (cq *CommandQueue) getOverlaps( readOnly bool, timestamp hlc.Timestamp, rng interval.Range, ) []*cmd { @@ -680,6 +686,7 @@ func (o *overlapHeap) Push(x interface{}) { func (o *overlapHeap) Pop() interface{} { n := len(*o) - 1 x := (*o)[n] + (*o)[n] = nil // for gc *o = (*o)[:n] return x }