Skip to content

Commit

Permalink
kvserver: avoid key heap allocation in multiSSTWriter.rolloverSST
Browse files Browse the repository at this point in the history
rolloverSST is called for every key-value pair in the incoming snapshot,
and the key parameter was inadvertently escaping to the heap. This was
20% of the number of allocations on a node in the 150 node cluster test.

Epic: none

Release note: None
  • Loading branch information
sumeerbhola committed Nov 7, 2024
1 parent 58dece5 commit 461f228
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions pkg/kv/kvserver/store_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ func (msstw *multiSSTWriter) initSST(ctx context.Context) error {
return nil
}

// NB: when nextKey is non-nil, do not do anything in this function to cause
// nextKey at the caller to escape to the heap.
func (msstw *multiSSTWriter) finalizeSST(ctx context.Context, nextKey *storage.EngineKey) error {
currSpan := msstw.currentSpan()
if msstw.currSpanIsMVCCSpan() {
Expand Down Expand Up @@ -310,32 +312,37 @@ func (msstw *multiSSTWriter) finalizeSST(ctx context.Context, nextKey *storage.E
if nextKey != nil {
meta := msstw.currSST.Meta
encodedNextKey := nextKey.Encode()
// Use nextKeyCopy for the remainder of this function. Calling
// errors.Errorf with nextKey caused it to escape to the heap in the
// caller of finalizeSST (even when finalizeSST was not called), which was
// costly.
nextKeyCopy := *nextKey
if meta.HasPointKeys && storage.EngineKeyCompare(meta.LargestPoint.UserKey, encodedNextKey) > 0 {
metaEndKey, ok := storage.DecodeEngineKey(meta.LargestPoint.UserKey)
if !ok {
return errors.Errorf("multiSSTWriter created overlapping ingestion sstables: sstable largest point key %s > next sstable start key %s",
meta.LargestPoint.UserKey, nextKey)
meta.LargestPoint.UserKey, nextKeyCopy)
}
return errors.Errorf("multiSSTWriter created overlapping ingestion sstables: sstable largest point key %s > next sstable start key %s",
metaEndKey, nextKey)
metaEndKey, nextKeyCopy)
}
if meta.HasRangeDelKeys && storage.EngineKeyCompare(meta.LargestRangeDel.UserKey, encodedNextKey) > 0 {
metaEndKey, ok := storage.DecodeEngineKey(meta.LargestRangeDel.UserKey)
if !ok {
return errors.Errorf("multiSSTWriter created overlapping ingestion sstables: sstable largest range del %s > next sstable start key %s",
meta.LargestRangeDel.UserKey, nextKey)
meta.LargestRangeDel.UserKey, nextKeyCopy)
}
return errors.Errorf("multiSSTWriter created overlapping ingestion sstables: sstable largest range del %s > next sstable start key %s",
metaEndKey, nextKey)
metaEndKey, nextKeyCopy)
}
if meta.HasRangeKeys && storage.EngineKeyCompare(meta.LargestRangeKey.UserKey, encodedNextKey) > 0 {
metaEndKey, ok := storage.DecodeEngineKey(meta.LargestRangeKey.UserKey)
if !ok {
return errors.Errorf("multiSSTWriter created overlapping ingestion sstables: sstable largest range key %s > next sstable start key %s",
meta.LargestRangeKey.UserKey, nextKey)
meta.LargestRangeKey.UserKey, nextKeyCopy)
}
return errors.Errorf("multiSSTWriter created overlapping ingestion sstables: sstable largest range key %s > next sstable start key %s",
metaEndKey, nextKey)
metaEndKey, nextKeyCopy)
}
}
// Account for any additional bytes written other than the KV data.
Expand Down

0 comments on commit 461f228

Please sign in to comment.