Merge #36549

36549: batcheval,bulk: move AddSSTable mvcc stats computation to caller r=dt a=dt This allows a client to include the MVCCStats for the SST when sending an AddSSTable request. If stats are included in a request, the receiving store can use the stapled stats instead of computing them itself. While this is still the same total amount of work, moving it to the client can help when many clients are producing and sending SSTs to the same range, which could then become a bottleneck. Additioanlly, this paves the way to future optimization that could actually reduce the total work done in some cases. While the server, handed an arbitrary SST, has to assume that it can contain anything and has to run the full recompute on it, in some cases, some clients may potentially be able to make simplifying assumptions based on what they are putting in their SST, and thus be able to compute or derive stats more cheapy than running all of `ComputeStatsGo`. Giving the client the option to pass its own stats provides this flexibility. Release note: None. Co-authored-by: David Taylor <[email protected]>
cockroachdb · Aug 4, 2019 · ea50f7c · ea50f7c
2 parents 4296ec0 + 1c702b0
commit ea50f7c
Show file tree

Hide file tree

Showing 13 changed files with 1,097 additions and 885 deletions.
diff --git a/c-deps/libroach/protos/roachpb/api.pb.cc b/c-deps/libroach/protos/roachpb/api.pb.cc
diff --git a/c-deps/libroach/protos/roachpb/api.pb.h b/c-deps/libroach/protos/roachpb/api.pb.h
diff --git a/pkg/ccl/importccl/bench_test.go b/pkg/ccl/importccl/bench_test.go
@@ -134,7 +134,7 @@ func benchmarkAddSSTable(b *testing.B, dir string, tables []tableSSTable) {
 		b.StartTimer()
 		for _, t := range tables {
 			totalBytes += int64(len(t.sstData))
-			require.NoError(b, kvDB.AddSSTable(ctx, t.span.Key, t.span.EndKey, t.sstData, true /* disallowShadowing */))
+			require.NoError(b, kvDB.AddSSTable(ctx, t.span.Key, t.span.EndKey, t.sstData, true /* disallowShadowing */, nil /* stats */))
 		}
 		b.StopTimer()
 

diff --git a/pkg/ccl/storageccl/bench_test.go b/pkg/ccl/storageccl/bench_test.go
@@ -77,7 +77,7 @@ func BenchmarkAddSSTable(b *testing.B) {
 				totalLen += int64(len(data))
 
 				b.StartTimer()
-				if err := kvDB.AddSSTable(ctx, span.Key, span.EndKey, data, false /* disallowShadowing */); err != nil {
+				if err := kvDB.AddSSTable(ctx, span.Key, span.EndKey, data, false /* disallowShadowing */, nil /* stats */); err != nil {
 					b.Fatalf("%+v", err)
 				}
 				b.StopTimer()

diff --git a/pkg/ccl/workloadccl/format/sstable.go b/pkg/ccl/workloadccl/format/sstable.go
@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 	"github.com/cockroachdb/cockroach/pkg/storage/bulk"
+	"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/workload"
@@ -105,7 +106,7 @@ func ToSSTable(t workload.Table, tableID sqlbase.ID, ts time.Time) ([]byte, erro
 type addSSTableSender [][]byte
 
 func (s *addSSTableSender) AddSSTable(
-	_ context.Context, _, _ interface{}, data []byte, _ bool,
+	_ context.Context, _, _ interface{}, data []byte, _ bool, _ *enginepb.MVCCStats,
 ) error {
 	*s = append(*s, data)
 	return nil

diff --git a/pkg/internal/client/batch.go b/pkg/internal/client/batch.go
@@ -14,6 +14,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/pkg/errors"
 )
@@ -705,7 +706,9 @@ func (b *Batch) writeBatch(s, e interface{}, data []byte) {
 }
 
 // addSSTable is only exported on DB.
-func (b *Batch) addSSTable(s, e interface{}, data []byte, disallowShadowing bool) {
+func (b *Batch) addSSTable(
+	s, e interface{}, data []byte, disallowShadowing bool, stats *enginepb.MVCCStats,
+) {
 	begin, err := marshalKey(s)
 	if err != nil {
 		b.initResult(0, 0, notRaw, err)
@@ -723,6 +726,7 @@ func (b *Batch) addSSTable(s, e interface{}, data []byte, disallowShadowing bool
 		},
 		Data:              data,
 		DisallowShadowing: disallowShadowing,
+		MVCCStats:         stats,
 	}
 	b.appendReqs(req)
 	b.initResult(1, 0, notRaw, nil)

diff --git a/pkg/internal/client/db.go b/pkg/internal/client/db.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -568,10 +569,14 @@ func (db *DB) WriteBatch(ctx context.Context, begin, end interface{}, data []byt
 // AddSSTable links a file into the RocksDB log-structured merge-tree. Existing
 // data in the range is cleared.
 func (db *DB) AddSSTable(
-	ctx context.Context, begin, end interface{}, data []byte, disallowShadowing bool,
+	ctx context.Context,
+	begin, end interface{},
+	data []byte,
+	disallowShadowing bool,
+	stats *enginepb.MVCCStats,
 ) error {
 	b := &Batch{}
-	b.addSSTable(begin, end, data, disallowShadowing)
+	b.addSSTable(begin, end, data, disallowShadowing, stats)
 	return getOneErr(db.Run(ctx, b), b)
 }