Skip to content

Commit

Permalink
storage: avoid per-kv allocations during consistency checks
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nvanbenschoten committed Aug 31, 2018
1 parent 9f1a5b5 commit a858aeb
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 a858aeb

Please sign in to comment.