Skip to content

Commit

Permalink
Merge #29419
Browse files Browse the repository at this point in the history
29419: storage: avoid per-kv allocations during consistency checks r=nvanbenschoten a=nvanbenschoten

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 golang/go#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 golang/go#27403 and search for any other offenders in our codebase. I already see a few.

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
  • Loading branch information
craig[bot] and nvanbenschoten committed Aug 31, 2018
2 parents 17df734 + a858aeb commit eab7171
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions pkg/storage/replica_consistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}

Expand Down

0 comments on commit eab7171

Please sign in to comment.