Skip to content

Commit

Permalink
storage: reduce allocations during fingerprinting
Browse files Browse the repository at this point in the history
If no rangekeys are encountered during
fingerprinting the underlying SSTWriter does not write
any data to the SST, but still produces a non-empty
SST file. This file contains an empty data block, properties and
a footer that amount to 795 bytes. Since the file does
not contain any rangekeys it is going to be thrown away
on the client side, and so ferrying this file in the
ExportResponse is wasteful.

Experiments on the c2c/kv0 roachtest showed that fingerprinting
a single range ~250MB would result in 600k+ empty files. This magnitude
of pagination was all attributable to the elastic CPU limiter that
preempts an ExportRequest if it is consuming more CPU than it
was allotted. 600k+ empty files meant that we were buffering close
to 500MB of ExportResponse_Files (600k * 790bytes) on the node
serving the ExportRequest. This was then shipped over grpc to the
client where we were opening a multi-mem iterator over these 600k+ empty
files causing a node with 15GiB RAM to OOM.

Since fingerprinting does not set a `TargetBytes` on the export requests
it sends, in a cluster with more than one ranage there could be several
requests concurrently buffering these empty files, resulting in an even
more pronounced memory blowup.

This change ensures that the MemFile is nil'ed out if there
are no rangekeys encountered during the fingerprinting.

This change also returns nil values for the `BulkOpSummary`
and `Span` field of the `ExportResponse` when generated as
part of fingerprinting. These fields are not used and show
up in memory profiles of the c2c/kv0 roachtest.

Release note: None

Informs: #93078
  • Loading branch information
adityamaru committed Jan 18, 2023
1 parent 51005e4 commit 260f4fa
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 28 deletions.
60 changes: 46 additions & 14 deletions pkg/kv/kvserver/batcheval/cmd_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,14 @@ func evalExport(
resumeKeyTS = args.ResumeKeyTS
}

maybeAnnotateExceedMaxSizeError := func(err error) error {
if errors.HasType(err, (*storage.ExceedMaxSizeError)(nil)) {
return errors.WithHintf(err,
"consider increasing cluster setting %q", MaxExportOverageSetting)
}
return err
}

var curSizeOfExportedSSTs int64
for start := args.Key; start != nil; {
destFile := &storage.MemFile{}
Expand All @@ -186,16 +194,26 @@ func evalExport(
StripTenantPrefix: true,
StripValueChecksum: true,
}
summary, resume, fingerprint, err = storage.MVCCExportFingerprint(ctx, cArgs.EvalCtx.ClusterSettings(), reader, opts, destFile)
var hasRangeKeys bool
summary, resume, fingerprint, hasRangeKeys, err = storage.MVCCExportFingerprint(ctx,
cArgs.EvalCtx.ClusterSettings(), reader, opts, destFile)
if err != nil {
return result.Result{}, maybeAnnotateExceedMaxSizeError(err)
}

// If no range keys were encountered during fingerprinting then we zero
// out the underlying SST file as there is no use in sending an empty file
// part of the ExportResponse. This frees up the memory used by the empty
// SST file.
if !hasRangeKeys {
destFile = &storage.MemFile{}
}
} else {
summary, resume, err = storage.MVCCExportToSST(ctx, cArgs.EvalCtx.ClusterSettings(), reader, opts, destFile)
}
if err != nil {
if errors.HasType(err, (*storage.ExceedMaxSizeError)(nil)) {
err = errors.WithHintf(err,
"consider increasing cluster setting %q", MaxExportOverageSetting)
summary, resume, err = storage.MVCCExportToSST(ctx, cArgs.EvalCtx.ClusterSettings(), reader,
opts, destFile)
if err != nil {
return result.Result{}, maybeAnnotateExceedMaxSizeError(err)
}
return result.Result{}, err
}
data := destFile.Data()

Expand All @@ -222,12 +240,26 @@ func evalExport(
} else {
span.EndKey = args.EndKey
}
exported := roachpb.ExportResponse_File{
Span: span,
EndKeyTS: resume.Timestamp,
Exported: summary,
SST: data,
Fingerprint: fingerprint,

var exported roachpb.ExportResponse_File
if args.ExportFingerprint {
// A fingerprinting ExportRequest does not need to return the
// BulkOpSummary or the exported Span. This is because we do not expect
// the sender of a fingerprint ExportRequest to use anything but the
// `Fingerprint` for point-keys and the SST file that contains the
// rangekeys we encountered during ExportRequest evaluation.
exported = roachpb.ExportResponse_File{
EndKeyTS: resume.Timestamp,
SST: data,
Fingerprint: fingerprint,
}
} else {
exported = roachpb.ExportResponse_File{
Span: span,
EndKeyTS: resume.Timestamp,
Exported: summary,
SST: data,
}
}
reply.Files = append(reply.Files, exported)
start = resume.Key
Expand Down
1 change: 0 additions & 1 deletion pkg/storage/fingerprint_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ func FingerprintRangekeys(
) (uint64, error) {
ctx, sp := tracing.ChildSpan(ctx, "storage.FingerprintRangekeys")
defer sp.Finish()
_ = ctx // ctx is currently unused, but this new ctx should be used below in the future.

if len(ssts) == 0 {
return 0, nil
Expand Down
20 changes: 14 additions & 6 deletions pkg/storage/mvcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -6151,10 +6151,13 @@ func MVCCIsSpanEmpty(
// Range keys are not fingerprinted but instead written to a pebble SST that is
// returned to the caller. This is because range keys do not have a stable,
// discrete identity and so it is up to the caller to define a deterministic
// fingerprinting scheme across all returned range keys.
// fingerprinting scheme across all returned range keys. The returned boolean
// indicates whether any rangekeys were encountered during the export, this bool
// is used by the caller to throw away the empty SST file and avoid unnecessary
// allocations.
func MVCCExportFingerprint(
ctx context.Context, cs *cluster.Settings, reader Reader, opts MVCCExportOptions, dest io.Writer,
) (roachpb.BulkOpSummary, MVCCKey, uint64, error) {
) (roachpb.BulkOpSummary, MVCCKey, uint64, bool, error) {
ctx, span := tracing.ChildSpan(ctx, "storage.MVCCExportFingerprint")
defer span.Finish()

Expand All @@ -6164,11 +6167,16 @@ func MVCCExportFingerprint(

summary, resumeKey, err := mvccExportToWriter(ctx, reader, opts, &fingerprintWriter)
if err != nil {
return roachpb.BulkOpSummary{}, MVCCKey{}, 0, err
return roachpb.BulkOpSummary{}, MVCCKey{}, 0, false, err
}

fingerprint, err := fingerprintWriter.Finish()
return summary, resumeKey, fingerprint, err
if err != nil {
return roachpb.BulkOpSummary{}, MVCCKey{}, 0, false, err
}

hasRangeKeys := fingerprintWriter.sstWriter.DataSize != 0
return summary, resumeKey, fingerprint, hasRangeKeys, err
}

// MVCCExportToSST exports changes to the keyrange [StartKey, EndKey) over the
Expand All @@ -6191,9 +6199,9 @@ func MVCCExportToSST(
// If no records were added to the sstable, skip
// completing it and return an empty summary.
//
// We still propogate the resumeKey because our
// We still propagate the resumeKey because our
// iteration may have been halted because of resource
// limitiations before any keys were added to the
// limitations before any keys were added to the
// returned SST.
return roachpb.BulkOpSummary{}, resumeKey, nil
}
Expand Down
14 changes: 11 additions & 3 deletions pkg/storage/mvcc_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1388,12 +1388,17 @@ func cmdExport(e *evalCtx) error {
var summary roachpb.BulkOpSummary
var resume storage.MVCCKey
var fingerprint uint64
var hasRangeKeys bool
var err error
if shouldFingerprint {
summary, resume, fingerprint, err = storage.MVCCExportFingerprint(e.ctx, e.st, r, opts, sstFile)
summary, resume, fingerprint, hasRangeKeys, err = storage.MVCCExportFingerprint(e.ctx, e.st, r,
opts, sstFile)
if err != nil {
return err
}
if !hasRangeKeys {
sstFile = &storage.MemFile{}
}
e.results.buf.Printf("export: %s", &summary)
e.results.buf.Print(" fingerprint=true")
} else {
Expand All @@ -1410,9 +1415,12 @@ func cmdExport(e *evalCtx) error {
e.results.buf.Printf("\n")

if shouldFingerprint {
var ssts [][]byte
if sstFile.Len() != 0 {
ssts = append(ssts, sstFile.Bytes())
}
// Fingerprint the rangekeys returned as a pebble SST.
rangekeyFingerprint, err := storage.FingerprintRangekeys(e.ctx, e.st, opts.FingerprintOptions,
[][]byte{sstFile.Bytes()})
rangekeyFingerprint, err := storage.FingerprintRangekeys(e.ctx, e.st, opts.FingerprintOptions, ssts)
if err != nil {
return err
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/storage/mvcc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6441,9 +6441,12 @@ func TestMVCCExportFingerprint(t *testing.T) {
fingerprint := func(opts MVCCExportOptions, engine Engine) (uint64, []byte, roachpb.BulkOpSummary, MVCCKey) {
dest := &MemFile{}
var err error
res, resumeKey, fingerprint, err := MVCCExportFingerprint(
res, resumeKey, fingerprint, hasRangeKeys, err := MVCCExportFingerprint(
ctx, st, engine, opts, dest)
require.NoError(t, err)
if !hasRangeKeys {
dest = &MemFile{}
}
return fingerprint, dest.Data(), res, resumeKey
}

Expand Down Expand Up @@ -6751,6 +6754,10 @@ func (f *fingerprintOracle) fingerprintPointKeys(t *testing.T, dataSST []byte) u
func getRangeKeys(t *testing.T, dataSST []byte) []MVCCRangeKeyStack {
t.Helper()

if len(dataSST) == 0 {
return []MVCCRangeKeyStack{}
}

iterOpts := IterOptions{
KeyTypes: IterKeyTypeRangesOnly,
LowerBound: keys.LocalMax,
Expand Down
3 changes: 0 additions & 3 deletions pkg/storage/sst_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
// SSTWriter writes SSTables.
type SSTWriter struct {
fw *sstable.Writer
f io.Writer
// DataSize tracks the total key and value bytes added so far.
DataSize int64
scratch []byte
Expand Down Expand Up @@ -101,7 +100,6 @@ func MakeBackupSSTWriter(ctx context.Context, cs *cluster.Settings, f io.Writer)
opts.MergerName = "nullptr"
return SSTWriter{
fw: sstable.NewWriter(noopSyncCloser{f}, opts),
f: f,
supportsRangeKeys: opts.TableFormat >= sstable.TableFormatPebblev2,
}
}
Expand All @@ -115,7 +113,6 @@ func MakeIngestionSSTWriter(
opts := MakeIngestionWriterOptions(ctx, cs)
return SSTWriter{
fw: sstable.NewWriter(f, opts),
f: f,
supportsRangeKeys: opts.TableFormat >= sstable.TableFormatPebblev2,
}
}
Expand Down

0 comments on commit 260f4fa

Please sign in to comment.