Skip to content

Commit 8cd345b

Browse files
craig[bot]tbg
andcommitted
Merge #144285
144285: kvserver: improve TestMultiSSTWriterInitSST r=tbg a=tbg Extracted/brushed up from #142653. Epic: CRDB-46488 Release note: None Co-authored-by: Tobias Grieger <[email protected]>
2 parents def17c2 + c5ad4f0 commit 8cd345b

File tree

6 files changed

+149
-129
lines changed

6 files changed

+149
-129
lines changed

pkg/kv/kvserver/replica_sst_snapshot_storage_test.go

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package kvserver
88
import (
99
"context"
1010
"encoding/binary"
11+
"fmt"
1112
"io"
1213
"path/filepath"
1314
"strconv"
@@ -21,6 +22,9 @@ import (
2122
"github.com/cockroachdb/cockroach/pkg/storage/fs"
2223
"github.com/cockroachdb/cockroach/pkg/storage/mvccencoding"
2324
"github.com/cockroachdb/cockroach/pkg/testutils"
25+
"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
26+
"github.com/cockroachdb/cockroach/pkg/testutils/echotest"
27+
"github.com/cockroachdb/cockroach/pkg/testutils/storageutils"
2428
"github.com/cockroachdb/cockroach/pkg/util/hlc"
2529
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
2630
"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -30,6 +34,7 @@ import (
3034
"github.com/cockroachdb/errors/oserror"
3135
"github.com/cockroachdb/pebble/sstable"
3236
"github.com/cockroachdb/pebble/vfs"
37+
"github.com/cockroachdb/redact"
3338
"github.com/stretchr/testify/require"
3439
"golang.org/x/time/rate"
3540
)
@@ -252,9 +257,10 @@ func TestSSTSnapshotStorageContextCancellation(t *testing.T) {
252257
require.ErrorIs(t, err, context.Canceled)
253258
}
254259

255-
// TestMultiSSTWriterInitSST tests that multiSSTWriter initializes each of the
256-
// SST files associated with the replicated key ranges by writing a range
257-
// deletion tombstone that spans the entire range of each respectively.
260+
// TestMultiSSTWriterInitSST tests the SSTS that multiSSTWriter generates.
261+
// In particular, certain SST files must contain range key deletes as well
262+
// as range deletes to make sure that ingesting the SST clears any existing
263+
// data.
258264
func TestMultiSSTWriterInitSST(t *testing.T) {
259265
defer leaktest.AfterTest(t)()
260266
defer log.Scope(t).Close(t)
@@ -294,26 +300,15 @@ func TestMultiSSTWriterInitSST(t *testing.T) {
294300
actualSSTs = append(actualSSTs, sst)
295301
}
296302

297-
// Construct an SST file for each of the key ranges and write a rangedel
298-
// tombstone that spans from Start to End.
299-
var expectedSSTs [][]byte
300-
for _, s := range keySpans {
301-
func() {
302-
sstFile := &storage.MemObject{}
303-
sst := storage.MakeIngestionSSTWriter(ctx, cluster.MakeTestingClusterSettings(), sstFile)
304-
defer sst.Close()
305-
err := sst.ClearRawRange(s.Key, s.EndKey, true, true)
306-
require.NoError(t, err)
307-
err = sst.Finish()
308-
require.NoError(t, err)
309-
expectedSSTs = append(expectedSSTs, sstFile.Data())
310-
}()
311-
}
303+
var buf redact.StringBuilder
304+
305+
_, _ = fmt.Fprintf(&buf, "writeBytes=%d sstSize=%d dataSize=%d\n", msstw.writeBytes, msstw.sstSize, msstw.dataSize)
312306

313-
require.Equal(t, len(actualSSTs), len(expectedSSTs))
314307
for i := range fileNames {
315-
require.Equal(t, actualSSTs[i], expectedSSTs[i])
308+
name := fmt.Sprintf("sst%d", i)
309+
require.NoError(t, storageutils.ReportSSTEntries(&buf, name, actualSSTs[i]))
316310
}
311+
echotest.Require(t, buf.String(), filepath.Join(datapathutils.TestDataPath(t, "echotest", t.Name())))
317312
}
318313

319314
func buildIterForScratch(
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
echo
2+
----
3+
writeBytes=4740 sstSize=4872 dataSize=132
4+
>> sst0:
5+
rangedel: /Local/RangeID/0/{r""-s""}
6+
rangekeydel: /Local/RangeID/0/{r""-s""}
7+
>> sst1:
8+
rangedel: /Local/Range{"d"-/Max}
9+
rangekeydel: /Local/Range{"d"-/Max}
10+
>> sst2:
11+
rangedel: /Local/Lock/Local/Range{"d"-/Max}
12+
rangekeydel: /Local/Lock/Local/Range{"d"-/Max}
13+
>> sst3:
14+
rangedel: /Local/Lock{"d"-/Max}
15+
rangekeydel: /Local/Lock{"d"-/Max}
16+
>> sst4:
17+
rangedel: {d-/Max}
18+
rangekeydel: {d-/Max}

pkg/storage/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ go_test(
188188
"//pkg/testutils/datapathutils",
189189
"//pkg/testutils/echotest",
190190
"//pkg/testutils/skip",
191+
"//pkg/testutils/storageutils",
191192
"//pkg/testutils/testfixtures",
192193
"//pkg/testutils/zerofields",
193194
"//pkg/util",

pkg/storage/mvcc_history_test.go

Lines changed: 2 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/cockroachdb/cockroach/pkg/storage/mvccencoding"
3434
"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
3535
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
36+
"github.com/cockroachdb/cockroach/pkg/testutils/storageutils"
3637
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3738
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
3839
"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -42,9 +43,6 @@ import (
4243
"github.com/cockroachdb/cockroach/pkg/util/uuid"
4344
"github.com/cockroachdb/datadriven"
4445
"github.com/cockroachdb/errors"
45-
"github.com/cockroachdb/pebble"
46-
"github.com/cockroachdb/pebble/sstable"
47-
"github.com/cockroachdb/pebble/sstable/block"
4846
"github.com/cockroachdb/pebble/vfs"
4947
"github.com/cockroachdb/redact"
5048
"github.com/stretchr/testify/require"
@@ -260,111 +258,6 @@ func TestMVCCHistories(t *testing.T) {
260258
return err
261259
}
262260

263-
// reportSSTEntries outputs entries from a raw SSTable. It uses a raw
264-
// SST iterator in order to accurately represent the raw SST data.
265-
reportSSTEntries := func(buf *redact.StringBuilder, name string, sst []byte) error {
266-
r, err := sstable.NewMemReader(sst, sstable.ReaderOptions{
267-
Comparer: &storage.EngineComparer,
268-
KeySchemas: sstable.MakeKeySchemas(storage.KeySchemas...),
269-
})
270-
if err != nil {
271-
return err
272-
}
273-
defer func() { _ = r.Close() }()
274-
buf.Printf(">> %s:\n", name)
275-
276-
// Dump point keys.
277-
iter, err := r.NewIter(sstable.NoTransforms, nil, nil, sstable.AssertNoBlobHandles)
278-
if err != nil {
279-
return err
280-
}
281-
defer func() { _ = iter.Close() }()
282-
for kv := iter.First(); kv != nil; kv = iter.Next() {
283-
if err := iter.Error(); err != nil {
284-
return err
285-
}
286-
key, err := storage.DecodeMVCCKey(kv.K.UserKey)
287-
if err != nil {
288-
return err
289-
}
290-
v, _, err := kv.Value(nil)
291-
if err != nil {
292-
return err
293-
}
294-
value, err := storage.DecodeMVCCValue(v)
295-
if err != nil {
296-
return err
297-
}
298-
buf.Printf("%s: %s -> %s\n", strings.ToLower(kv.Kind().String()), key, value)
299-
}
300-
301-
// Dump rangedels.
302-
if rdIter, err := r.NewRawRangeDelIter(context.Background(), block.NoFragmentTransforms, block.NoReadEnv); err != nil {
303-
return err
304-
} else if rdIter != nil {
305-
defer rdIter.Close()
306-
s, err := rdIter.First()
307-
for ; s != nil; s, err = rdIter.Next() {
308-
start, err := storage.DecodeMVCCKey(s.Start)
309-
if err != nil {
310-
return err
311-
}
312-
end, err := storage.DecodeMVCCKey(s.End)
313-
if err != nil {
314-
return err
315-
}
316-
for _, k := range s.Keys {
317-
buf.Printf("%s: %s\n", strings.ToLower(k.Kind().String()),
318-
roachpb.Span{Key: start.Key, EndKey: end.Key})
319-
}
320-
}
321-
if err != nil {
322-
return err
323-
}
324-
}
325-
326-
// Dump range keys.
327-
if rkIter, err := r.NewRawRangeKeyIter(context.Background(), block.NoFragmentTransforms, block.NoReadEnv); err != nil {
328-
return err
329-
} else if rkIter != nil {
330-
defer rkIter.Close()
331-
s, err := rkIter.First()
332-
for ; s != nil; s, err = rkIter.Next() {
333-
start, err := storage.DecodeMVCCKey(s.Start)
334-
if err != nil {
335-
return err
336-
}
337-
end, err := storage.DecodeMVCCKey(s.End)
338-
if err != nil {
339-
return err
340-
}
341-
for _, k := range s.Keys {
342-
buf.Printf("%s: %s", strings.ToLower(k.Kind().String()),
343-
roachpb.Span{Key: start.Key, EndKey: end.Key})
344-
if len(k.Suffix) > 0 {
345-
ts, err := mvccencoding.DecodeMVCCTimestampSuffix(k.Suffix)
346-
if err != nil {
347-
return err
348-
}
349-
buf.Printf("/%s", ts)
350-
}
351-
if k.Kind() == pebble.InternalKeyKindRangeKeySet {
352-
value, err := storage.DecodeMVCCValue(k.Value)
353-
if err != nil {
354-
return err
355-
}
356-
buf.Printf(" -> %s", value)
357-
}
358-
buf.Printf("\n")
359-
}
360-
}
361-
if err != nil {
362-
return err
363-
}
364-
}
365-
return nil
366-
}
367-
368261
// reportLockTable outputs the contents of the lock table.
369262
reportLockTable := func(e *evalCtx, buf *redact.StringBuilder) error {
370263
// Replicated locks.
@@ -542,7 +435,7 @@ func TestMVCCHistories(t *testing.T) {
542435
}
543436
}
544437
for i, sst := range e.ssts {
545-
err = reportSSTEntries(&buf, fmt.Sprintf("sst-%d", i), sst)
438+
err = storageutils.ReportSSTEntries(&buf, fmt.Sprintf("sst-%d", i), sst)
546439
if err != nil {
547440
if foundErr == nil {
548441
// Handle the error below.

pkg/testutils/storageutils/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ go_library(
2626
"//pkg/util/syncutil",
2727
"//pkg/util/syncutil/singleflight",
2828
"@com_github_cockroachdb_pebble//:pebble",
29+
"@com_github_cockroachdb_pebble//sstable",
30+
"@com_github_cockroachdb_pebble//sstable/block",
31+
"@com_github_cockroachdb_redact//:redact",
2932
"@com_github_stretchr_testify//require",
3033
],
3134
)

pkg/testutils/storageutils/sst.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,20 @@ package storageutils
77

88
import (
99
"context"
10+
"strings"
1011
"testing"
1112

1213
"github.com/cockroachdb/cockroach/pkg/keys"
1314
"github.com/cockroachdb/cockroach/pkg/roachpb"
1415
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1516
"github.com/cockroachdb/cockroach/pkg/storage"
1617
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
18+
"github.com/cockroachdb/cockroach/pkg/storage/mvccencoding"
1719
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
1820
"github.com/cockroachdb/pebble"
21+
"github.com/cockroachdb/pebble/sstable"
22+
"github.com/cockroachdb/pebble/sstable/block"
23+
"github.com/cockroachdb/redact"
1924
"github.com/stretchr/testify/require"
2025
)
2126

@@ -121,3 +126,108 @@ func KeysFromSST(t *testing.T, data []byte) ([]storage.MVCCKey, []storage.MVCCRa
121126
}
122127
return results, rangeKeyRes
123128
}
129+
130+
// ReportSSTEntries iterates through an SST and dumps the raw SST data into the
131+
// buffer in a format suitable for datadriven testing output.
132+
func ReportSSTEntries(buf *redact.StringBuilder, name string, sst []byte) error {
133+
r, err := sstable.NewMemReader(sst, sstable.ReaderOptions{
134+
Comparer: &storage.EngineComparer,
135+
KeySchemas: sstable.MakeKeySchemas(storage.KeySchemas...),
136+
})
137+
if err != nil {
138+
return err
139+
}
140+
defer func() { _ = r.Close() }()
141+
buf.Printf(">> %s:\n", name)
142+
143+
// Dump point keys.
144+
iter, err := r.NewIter(sstable.NoTransforms, nil, nil, sstable.AssertNoBlobHandles)
145+
if err != nil {
146+
return err
147+
}
148+
defer func() { _ = iter.Close() }()
149+
for kv := iter.First(); kv != nil; kv = iter.Next() {
150+
if err := iter.Error(); err != nil {
151+
return err
152+
}
153+
key, err := storage.DecodeMVCCKey(kv.K.UserKey)
154+
if err != nil {
155+
return err
156+
}
157+
v, _, err := kv.Value(nil)
158+
if err != nil {
159+
return err
160+
}
161+
value, err := storage.DecodeMVCCValue(v)
162+
if err != nil {
163+
return err
164+
}
165+
buf.Printf("%s: %s -> %s\n", strings.ToLower(kv.Kind().String()), key, value)
166+
}
167+
168+
// Dump rangedels.
169+
if rdIter, err := r.NewRawRangeDelIter(context.Background(), block.NoFragmentTransforms, block.NoReadEnv); err != nil {
170+
return err
171+
} else if rdIter != nil {
172+
defer rdIter.Close()
173+
s, err := rdIter.First()
174+
for ; s != nil; s, err = rdIter.Next() {
175+
start, err := storage.DecodeMVCCKey(s.Start)
176+
if err != nil {
177+
return err
178+
}
179+
end, err := storage.DecodeMVCCKey(s.End)
180+
if err != nil {
181+
return err
182+
}
183+
for _, k := range s.Keys {
184+
buf.Printf("%s: %s\n", strings.ToLower(k.Kind().String()),
185+
roachpb.Span{Key: start.Key, EndKey: end.Key})
186+
}
187+
}
188+
if err != nil {
189+
return err
190+
}
191+
}
192+
193+
// Dump range keys.
194+
if rkIter, err := r.NewRawRangeKeyIter(context.Background(), block.NoFragmentTransforms, block.NoReadEnv); err != nil {
195+
return err
196+
} else if rkIter != nil {
197+
defer rkIter.Close()
198+
s, err := rkIter.First()
199+
for ; s != nil; s, err = rkIter.Next() {
200+
start, err := storage.DecodeMVCCKey(s.Start)
201+
if err != nil {
202+
return err
203+
}
204+
end, err := storage.DecodeMVCCKey(s.End)
205+
if err != nil {
206+
return err
207+
}
208+
for _, k := range s.Keys {
209+
buf.Printf("%s: %s", strings.ToLower(k.Kind().String()),
210+
roachpb.Span{Key: start.Key, EndKey: end.Key})
211+
if len(k.Suffix) > 0 {
212+
ts, err := mvccencoding.DecodeMVCCTimestampSuffix(k.Suffix)
213+
if err != nil {
214+
return err
215+
}
216+
buf.Printf("/%s", ts)
217+
}
218+
if k.Kind() == pebble.InternalKeyKindRangeKeySet {
219+
value, err := storage.DecodeMVCCValue(k.Value)
220+
if err != nil {
221+
return err
222+
}
223+
buf.Printf(" -> %s", value)
224+
}
225+
buf.Printf("\n")
226+
}
227+
}
228+
if err != nil {
229+
return err
230+
}
231+
}
232+
return nil
233+
}

0 commit comments

Comments
 (0)