Skip to content

Commit 0c5af8b

Browse files
committed
kv/bulk: turn on bulkio.ingest.compute_stats_diff_in_stream_batcher.enabled
We figured out the current cause of ComputeStatsDiff violations were due mid key sized based flushes, which are unavoidable and rare. Therefore, we feel good turning on accurate stats computing for PCR. This patch also adds logging every time we conduct a mid key flush. Fixes #152536 Release note: none
1 parent e9f420b commit 0c5af8b

File tree

2 files changed

+12
-8
lines changed

2 files changed

+12
-8
lines changed

pkg/kv/bulk/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ go_library(
3434
"//pkg/util/humanizeutil",
3535
"//pkg/util/limit",
3636
"//pkg/util/log",
37-
"//pkg/util/metamorphic",
3837
"//pkg/util/metric",
3938
"//pkg/util/mon",
4039
"//pkg/util/retry",

pkg/kv/bulk/sst_batcher.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3131
"github.com/cockroachdb/cockroach/pkg/util/limit"
3232
"github.com/cockroachdb/cockroach/pkg/util/log"
33-
"github.com/cockroachdb/cockroach/pkg/util/metamorphic"
3433
"github.com/cockroachdb/cockroach/pkg/util/mon"
3534
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
3635
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -70,7 +69,7 @@ var (
7069
settings.ApplicationLevel,
7170
"bulkio.ingest.compute_stats_diff_in_stream_batcher.enabled",
7271
"if set, kvserver will compute an accurate stats diff for every addsstable request",
73-
metamorphic.ConstantWithTestBool("computeStatsDiffInStreamBatcher", false),
72+
true,
7473
)
7574

7675
sstBatcherElasticCPUControlEnabled = settings.RegisterBoolSetting(
@@ -583,15 +582,21 @@ func (b *SSTBatcher) flushIfNeeded(ctx context.Context, nextKey roachpb.Key) err
583582
// That said, only do this if we are only moderately over the flush target;
584583
// if we are subtantially over the limit, just flush the partial row as we
585584
// cannot buffer indefinitely.
585+
586+
prevRow, prevErr := keys.EnsureSafeSplitKey(b.batch.endKey)
587+
nextRow, nextErr := keys.EnsureSafeSplitKey(nextKey)
588+
589+
// An error decoding either key implies it is not a valid row key and thus
590+
// not the same row for our purposes; we don't care what the error is.
591+
midKey := prevErr == nil && nextErr == nil && bytes.Equal(prevRow, nextRow)
586592
if b.batch.sstWriter.DataSize < 2*flushLimit {
587-
prevRow, prevErr := keys.EnsureSafeSplitKey(b.batch.endKey)
588-
nextRow, nextErr := keys.EnsureSafeSplitKey(nextKey)
589-
if prevErr == nil && nextErr == nil && bytes.Equal(prevRow, nextRow) {
590-
// An error decoding either key implies it is not a valid row key and thus
591-
// not the same row for our purposes; we don't care what the error is.
593+
if midKey {
592594
return nil // keep going to row boundary.
593595
}
594596
}
597+
if midKey {
598+
log.Dev.Infof(ctx, "flushing sst mid key %s due to size", b.batch.endKey)
599+
}
595600

596601
if b.mustSyncBeforeFlush {
597602
err := b.syncFlush()

0 commit comments

Comments
 (0)