From a858aebae48fbcb5e07e77f12f1064adadb8a16c Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Fri, 31 Aug 2018 02:49:25 -0400 Subject: [PATCH] storage: avoid per-kv allocations during consistency checks I noticed in an `alloc_objects` heap profile on a 3-node cluster restoring tpcc that more than 46% of all allocations were made in `computeChecksumPostApply`. Specifically, these allocations were all made in `Replica.sha512`. 28% of allocations were due to protobuf marshaling of `hlc.LegacyTimestamp`. The other 18% was in `encoding/binary.Write`. This removes both of these sources of per-key allocations. The first allocation was avoided by sharing a byte buffer across protobuf marshals. The second was avoided by removing the call to `binary.Write` (see https://github.com/golang/go/issues/27403). I confirmed that this is no longer an issue by looking at heap profiles from before and after in a test that performed a consistency check. I plan to follow up on https://github.com/golang/go/issues/27403 and search for any other offenders in our codebase. I already see a few. Release note: None --- pkg/storage/replica_consistency.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/storage/replica_consistency.go b/pkg/storage/replica_consistency.go index 6cd37c58449e..812948a026d9 100644 --- a/pkg/storage/replica_consistency.go +++ b/pkg/storage/replica_consistency.go @@ -360,9 +360,11 @@ func (r *Replica) sha512( defer iter.Close() var alloc bufalloc.ByteAllocator + var intBuf [8]byte + var legacyTimestamp hlc.LegacyTimestamp + var timestampBuf []byte hasher := sha512.New() - var legacyTimestamp hlc.LegacyTimestamp visitor := func(unsafeKey engine.MVCCKey, unsafeValue []byte) error { if snapshot != nil { // Add (a copy of) the kv pair into the debug message. @@ -375,24 +377,30 @@ func (r *Replica) sha512( } // Encode the length of the key and value. - if err := binary.Write(hasher, binary.LittleEndian, int64(len(unsafeKey.Key))); err != nil { + binary.LittleEndian.PutUint64(intBuf[:], uint64(len(unsafeKey.Key))) + if _, err := hasher.Write(intBuf[:]); err != nil { return err } - if err := binary.Write(hasher, binary.LittleEndian, int64(len(unsafeValue))); err != nil { + binary.LittleEndian.PutUint64(intBuf[:], uint64(len(unsafeValue))) + if _, err := hasher.Write(intBuf[:]); err != nil { return err } if _, err := hasher.Write(unsafeKey.Key); err != nil { return err } legacyTimestamp = hlc.LegacyTimestamp(unsafeKey.Timestamp) - timestamp, err := protoutil.Marshal(&legacyTimestamp) - if err != nil { + if size := legacyTimestamp.Size(); size > cap(timestampBuf) { + timestampBuf = make([]byte, size) + } else { + timestampBuf = timestampBuf[:size] + } + if _, err := protoutil.MarshalToWithoutFuzzing(&legacyTimestamp, timestampBuf); err != nil { return err } - if _, err := hasher.Write(timestamp); err != nil { + if _, err := hasher.Write(timestampBuf); err != nil { return err } - _, err = hasher.Write(unsafeValue) + _, err := hasher.Write(unsafeValue) return err }