From d6d394bf5c4974d79e21efe8c03f65ebf0bc10fa Mon Sep 17 00:00:00 2001 From: Jordan Lewis Date: Sat, 29 May 2021 15:11:57 -0300 Subject: [PATCH] row: release memory more eagerly in txnKVFetcher Previously, we could leave some dangling references to batch responses around in the txnKVFetcher when we were fetching more than one batch at a time. This would cause a delay in reclamation of memory for the lifetime of a given query. Release note (bug fix): use less memory in some queries, primarily lookup joins. --- pkg/sql/row/kv_batch_fetcher.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/sql/row/kv_batch_fetcher.go b/pkg/sql/row/kv_batch_fetcher.go index ba2ecb975397..a2452bc2eda1 100644 --- a/pkg/sql/row/kv_batch_fetcher.go +++ b/pkg/sql/row/kv_batch_fetcher.go @@ -516,35 +516,42 @@ func (f *txnKVFetcher) fetch(ctx context.Context) error { return nil } +// popBatch returns the 0th byte slice in a slice of byte slices, as well as +// the rest of the slice of the byte slices. It nils the pointer to the 0th +// element before reslicing the outer slice. +func popBatch(batches [][]byte) (batch []byte, remainingBatches [][]byte) { + batch, remainingBatches = batches[0], batches[1:] + batches[0] = nil + return batch, remainingBatches +} + // nextBatch returns the next batch of key/value pairs. If there are none // available, a fetch is initiated. When there are no more keys, ok is false. // origSpan returns the span that batch was fetched from, and bounds all of the // keys returned. func (f *txnKVFetcher) nextBatch( ctx context.Context, -) (ok bool, kvs []roachpb.KeyValue, batchResponse []byte, origSpan roachpb.Span, err error) { +) (ok bool, kvs []roachpb.KeyValue, batchResp []byte, origSpan roachpb.Span, err error) { if len(f.remainingBatches) > 0 { - batch := f.remainingBatches[0] - f.remainingBatches = f.remainingBatches[1:] - return true, nil, batch, f.origSpan, nil + batchResp, f.remainingBatches = popBatch(f.remainingBatches) + return true, nil, batchResp, f.origSpan, nil } for len(f.responses) > 0 { reply := f.responses[0].GetInner() + f.responses[0] = roachpb.ResponseUnion{} f.responses = f.responses[1:] origSpan := f.requestSpans[0] + f.requestSpans[0] = roachpb.Span{} f.requestSpans = f.requestSpans[1:] - var batchResp []byte switch t := reply.(type) { case *roachpb.ScanResponse: if len(t.BatchResponses) > 0 { - batchResp = t.BatchResponses[0] - f.remainingBatches = t.BatchResponses[1:] + batchResp, f.remainingBatches = popBatch(t.BatchResponses) } return true, t.Rows, batchResp, origSpan, nil case *roachpb.ReverseScanResponse: if len(t.BatchResponses) > 0 { - batchResp = t.BatchResponses[0] - f.remainingBatches = t.BatchResponses[1:] + batchResp, f.remainingBatches = popBatch(t.BatchResponses) } return true, t.Rows, batchResp, origSpan, nil case *roachpb.GetResponse: @@ -571,5 +578,7 @@ func (f *txnKVFetcher) nextBatch( // close releases the resources of this txnKVFetcher. func (f *txnKVFetcher) close(ctx context.Context) { + f.responses = nil + f.remainingBatches = nil f.acc.Close(ctx) }