Skip to content

Commit 6d59c06

Browse files
RaduBerindeRaduBerinde
committed
sstable: standardize format argument
There are multiple datadriven arguments with different values for specifying the table format. This change standardizes all of them to use `table-format`.
1 parent 30fc641 commit 6d59c06

19 files changed

+97
-195
lines changed

‎data_test.go‎

Lines changed: 17 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -584,40 +584,21 @@ func runBuildRemoteCmd(td *datadriven.TestData, d *DB, storage remote.Storage) e
584584
}
585585
path := td.CmdArgs[0].String()
586586

587-
// Override table format, if provided.
588-
tableFormat := d.TableFormat()
589-
var blockSize int64
590-
for _, cmdArg := range td.CmdArgs[1:] {
591-
switch cmdArg.Key {
592-
case "format":
593-
switch cmdArg.Vals[0] {
594-
case "pebblev1":
595-
tableFormat = sstable.TableFormatPebblev1
596-
case "pebblev2":
597-
tableFormat = sstable.TableFormatPebblev2
598-
case "pebblev3":
599-
tableFormat = sstable.TableFormatPebblev3
600-
case "pebblev4":
601-
tableFormat = sstable.TableFormatPebblev4
602-
default:
603-
return errors.Errorf("unknown format string %s", cmdArg.Vals[0])
604-
}
605-
case "block-size":
606-
var err error
607-
blockSize, err = strconv.ParseInt(cmdArg.Vals[0], 10, 64)
608-
if err != nil {
609-
return errors.Wrap(err, td.Pos)
610-
}
611-
}
612-
}
613-
614-
writeOpts := d.opts.MakeWriterOptions(0 /* level */, tableFormat)
615-
if blockSize == 0 && rand.IntN(4) == 0 {
616-
// Force two-level indexes if not already forced on or off.
617-
blockSize = 5
587+
// Use TableFormatMax here and downgrade after, if necessary. This ensures
588+
// that all fields are set.
589+
writeOpts := d.opts.MakeWriterOptions(0 /* level */, d.TableFormat())
590+
if rand.IntN(4) == 0 {
591+
// If block size is not specified (in which case these fields will be
592+
// overridden by ParseWriterOptions), force two-level indexes some of the
593+
// time.
594+
writeOpts.BlockSize = 5
595+
writeOpts.IndexBlockSize = 5
596+
}
597+
598+
// Now parse the arguments again against the real options.
599+
if err := sstable.ParseWriterOptions(&writeOpts, td.CmdArgs...); err != nil {
600+
return err
618601
}
619-
writeOpts.BlockSize = int(blockSize)
620-
writeOpts.IndexBlockSize = int(blockSize)
621602

622603
f, err := storage.CreateObject(path)
623604
if err != nil {
@@ -707,29 +688,10 @@ func runBuildCmd(
707688
}
708689
path := td.CmdArgs[0].String()
709690

710-
// Override table format, if provided.
711-
tableFormat := d.TableFormat()
712-
for _, cmdArg := range td.CmdArgs[1:] {
713-
switch cmdArg.Key {
714-
case "format":
715-
switch cmdArg.Vals[0] {
716-
case "pebblev1":
717-
tableFormat = sstable.TableFormatPebblev1
718-
case "pebblev2":
719-
tableFormat = sstable.TableFormatPebblev2
720-
case "pebblev3":
721-
tableFormat = sstable.TableFormatPebblev3
722-
case "pebblev4":
723-
tableFormat = sstable.TableFormatPebblev4
724-
case "pebblev5":
725-
tableFormat = sstable.TableFormatPebblev5
726-
default:
727-
return errors.Errorf("unknown format string %s", cmdArg.Vals[0])
728-
}
729-
}
691+
writeOpts := d.opts.MakeWriterOptions(0 /* level */, d.TableFormat())
692+
if err := sstable.ParseWriterOptions(&writeOpts, td.CmdArgs[1:]...); err != nil {
693+
return err
730694
}
731-
732-
writeOpts := d.opts.MakeWriterOptions(0 /* level */, tableFormat)
733695
var blobReferences blobtest.References
734696
f, err := fs.Create(path, vfs.WriteCategoryUnspecified)
735697
if err != nil {

‎level_iter_test.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,21 +229,19 @@ func (lt *levelIterTest) runBuild(d *datadriven.TestData) string {
229229
return err.Error()
230230
}
231231

232-
tableFormat := sstable.TableFormatMinSupported
233-
for _, arg := range d.CmdArgs {
234-
if arg.Key == "format" {
235-
switch arg.Vals[0] {
236-
case "pebblev2":
237-
tableFormat = sstable.TableFormatPebblev2
238-
}
239-
}
240-
}
241232
fp := bloom.FilterPolicy(10)
242-
w := sstable.NewRawWriter(objstorageprovider.NewFileWritable(f0), sstable.WriterOptions{
233+
writerOpts := sstable.WriterOptions{
243234
Comparer: &lt.cmp,
244235
FilterPolicy: fp,
245-
TableFormat: tableFormat,
246-
})
236+
}
237+
if err := sstable.ParseWriterOptions(&writerOpts, d.CmdArgs...); err != nil {
238+
return err.Error()
239+
}
240+
if writerOpts.TableFormat == 0 {
241+
writerOpts.TableFormat = sstable.TableFormatMinSupported
242+
}
243+
244+
w := sstable.NewRawWriter(objstorageprovider.NewFileWritable(f0), writerOpts)
247245
var tombstones []keyspan.Span
248246
f := keyspan.Fragmenter{
249247
Cmp: lt.cmp.Compare,

‎sstable/copier_test.go‎

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package sstable
77
import (
88
"context"
99
"fmt"
10-
"strconv"
1110
"strings"
1211
"testing"
1312

@@ -43,40 +42,17 @@ func TestCopySpan(t *testing.T) {
4342
fileNameToNum[d.CmdArgs[0].Key] = nextFileNum
4443
nextFileNum++
4544
tableFormat := TableFormatMax
46-
blockSize := 1
47-
var indexBlockSize int
48-
for i := range d.CmdArgs[1:] {
49-
switch d.CmdArgs[i+1].Key {
50-
case "format":
51-
switch d.CmdArgs[i+1].Vals[0] {
52-
case "pebblev4":
53-
tableFormat = TableFormatPebblev4
54-
case "pebblev5":
55-
tableFormat = TableFormatPebblev5
56-
case "pebblev6":
57-
tableFormat = TableFormatPebblev6
58-
}
59-
case "block_size":
60-
var err error
61-
blockSize, err = strconv.Atoi(d.CmdArgs[i+1].FirstVal(t))
62-
if err != nil {
63-
return err.Error()
64-
}
65-
case "index_block_size":
66-
var err error
67-
indexBlockSize, err = strconv.Atoi(d.CmdArgs[i+1].FirstVal(t))
68-
if err != nil {
69-
return err.Error()
70-
}
71-
}
45+
46+
writerOpts := WriterOptions{
47+
BlockSize: 1,
48+
TableFormat: tableFormat,
49+
Comparer: testkeys.Comparer,
50+
KeySchema: &keySchema,
7251
}
73-
w := NewWriter(objstorageprovider.NewFileWritable(f), WriterOptions{
74-
BlockSize: blockSize,
75-
IndexBlockSize: indexBlockSize,
76-
TableFormat: tableFormat,
77-
Comparer: testkeys.Comparer,
78-
KeySchema: &keySchema,
79-
})
52+
if err := ParseWriterOptions(&writerOpts, d.CmdArgs[1:]...); err != nil {
53+
t.Fatal(err)
54+
}
55+
w := NewWriter(objstorageprovider.NewFileWritable(f), writerOpts)
8056
for _, key := range strings.Split(d.Input, "\n") {
8157
j := strings.Index(key, ":")
8258
ikey := base.ParseInternalKey(key[:j])

‎sstable/reader_test.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,6 @@ func runVirtualReaderTest(t *testing.T, path string, blockSize, indexBlockSize i
168168
NewTestKeysBlockPropertyCollector,
169169
},
170170
}
171-
if arg, ok := td.Arg("table-format"); ok {
172-
// The datadriven cmd parser will parse the TableFormat string
173-
// because its string representation looks like the datadriven
174-
// format for multiple arguments (<arg1>,<arg2>).
175-
name, v := arg.TwoVals(t)
176-
writerOpts.TableFormat, err = ParseTableFormatString(fmt.Sprintf("(%s,%s)", name, v))
177-
if err != nil {
178-
t.Fatal(err)
179-
}
180-
}
181171
wMeta, r, err = runBuildCmd(td, writerOpts, nil /* cacheHandle */)
182172
if err != nil {
183173
return err.Error()

‎sstable/test_utils.go‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ func decodeBlobInlineHandleAndAttribute(
264264
// ParseWriterOptions modifies WriterOptions based on the given arguments. Each
265265
// argument is a string or a fmt.Stringer (like datadriven.TestData.CmdArg) with
266266
// format either "<key>" or "<key>=<value>".
267+
//
268+
// Note that the test can specify a table format. If a format is already
269+
// specified in WriterOptions, the test format must not be newer than that.
267270
func ParseWriterOptions[StringOrStringer any](o *WriterOptions, args ...StringOrStringer) error {
268271
for _, arg := range args {
269272
str, ok := any(arg).(string)
@@ -273,27 +276,17 @@ func ParseWriterOptions[StringOrStringer any](o *WriterOptions, args ...StringOr
273276
key, value, _ := strings.Cut(str, "=")
274277
var err error
275278
switch key {
276-
case "leveldb":
277-
o.TableFormat = TableFormatLevelDB
278-
case "format":
279-
switch value {
280-
case "leveldb":
281-
o.TableFormat = TableFormatLevelDB
282-
case "pebblev1":
283-
o.TableFormat = TableFormatPebblev1
284-
case "pebblev2":
285-
o.TableFormat = TableFormatPebblev2
286-
case "pebblev3":
287-
o.TableFormat = TableFormatPebblev3
288-
case "pebblev4":
289-
o.TableFormat = TableFormatPebblev4
290-
case "pebblev5":
291-
o.TableFormat = TableFormatPebblev5
292-
case "pebblev6":
293-
o.TableFormat = TableFormatPebblev6
294-
default:
295-
return errors.Errorf("unknown format string %s", value)
279+
case "table-format":
280+
var tableFormat TableFormat
281+
tableFormat, err = ParseTableFormatString("(" + value + ")")
282+
if err != nil {
283+
return err
284+
}
285+
if o.TableFormat != 0 && o.TableFormat < tableFormat {
286+
return errors.Errorf("table format %s is newer than default format %s", tableFormat, o.TableFormat)
296287
}
288+
o.TableFormat = tableFormat
289+
297290
case "block-size":
298291
o.BlockSize, err = strconv.Atoi(value)
299292

@@ -312,6 +305,9 @@ func ParseWriterOptions[StringOrStringer any](o *WriterOptions, args ...StringOr
312305
case "is-strict-obsolete":
313306
o.IsStrictObsolete = true
314307

308+
case "format", "leveldb":
309+
return errors.Errorf("%q is deprecated", key)
310+
315311
default:
316312
// TODO(radu): ignoring unknown keys is error-prone; we need to find an
317313
// easy way for the upper layer to extract its own arguments.

‎sstable/testdata/copy_span‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
# Simple test case with row blocks.
33

4-
build test1 format=pebblev4
4+
build test1 table-format=Pebble,v4
55
a.SET.5:foo
66
b.SET.3:bar
77
c.SET.4:baz
@@ -37,7 +37,7 @@ c#0,SET: baz
3737

3838

3939
# Try the above with small blocks.
40-
build test22 format=pebblev4 block_size=1 index_block_size=1
40+
build test22 table-format=Pebble,v4 block_size=1 index_block_size=1
4141
a.SET.5:foo
4242
b.SET.3:bar
4343
c.SET.4:baz
@@ -85,7 +85,7 @@ c#0,SET: baz
8585

8686
# Try the above with columnar blocks.
8787

88-
build test3 format=pebblev5
88+
build test3 table-format=Pebble,v5
8989
a.SET.5:foo
9090
b.SET.3:bar
9191
c.SET.4:baz
@@ -119,7 +119,7 @@ c#0,SET: baz
119119

120120

121121
# Try the above with small blocks.
122-
build test32 format=pebblev5 block_size=1 index_block_size=1
122+
build test32 table-format=Pebble,v5 block_size=1 index_block_size=1
123123
a.SET.5:foo
124124
b.SET.3:bar
125125
c.SET.4:baz
@@ -159,7 +159,7 @@ c#0,SET: baz
159159

160160
# Try the above with checksummed columnar blocks.
161161

162-
build test3 format=pebblev6
162+
build test3 table-format=Pebble,v6
163163
a.SET.5:foo
164164
b.SET.3:bar
165165
c.SET.4:baz
@@ -193,7 +193,7 @@ c#0,SET: baz
193193

194194

195195
# Try the above with small blocks.
196-
build test32 format=pebblev6 block_size=1 index_block_size=1
196+
build test32 table-format=Pebble,v6 block_size=1 index_block_size=1
197197
a.SET.5:foo
198198
b.SET.3:bar
199199
c.SET.4:baz

‎sstable/testdata/virtual_reader_props‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Simple sanity test with single level index.
2-
build table-format=(Pebble,v4)
2+
build table-format=Pebble,v4
33
a.SET.1:a
44
b.SET.1:b
55
c.SET.1:c
@@ -26,7 +26,7 @@ props:
2626

2727
# Repeat the above with (Pebble,v5).
2828

29-
build table-format=(Pebble,v5)
29+
build table-format=Pebble,v5
3030
a.SET.1:a
3131
b.SET.1:b
3232
c.SET.1:c
@@ -52,7 +52,7 @@ props:
5252

5353

5454
# Test 2: Similar to test 1 but force two level iterators.
55-
build block-size=1 index-block-size=1 table-format=(Pebble,v4)
55+
build block-size=1 index-block-size=1 table-format=Pebble,v4
5656
a.SET.1:a
5757
b.SET.1:b
5858
c.SET.1:c
@@ -127,7 +127,7 @@ b,aa,false
127127

128128
# Repeat the above with (Pebble,v5).
129129

130-
build block-size=1 index-block-size=1 table-format=(Pebble,v5)
130+
build block-size=1 index-block-size=1 table-format=Pebble,v5
131131
a.SET.1:a
132132
b.SET.1:b
133133
c.SET.1:c
@@ -200,7 +200,7 @@ constrain a,aa,false
200200
----
201201
b,aa,false
202202

203-
build block-size=1 index-block-size=1 table-format=(Pebble,v4)
203+
build block-size=1 index-block-size=1 table-format=Pebble,v4
204204
a.SET.1:a
205205
d.SET.2:d
206206
f.SET.3:f
@@ -232,7 +232,7 @@ props:
232232

233233
# Repeat the above with (Pebble,v5).
234234

235-
build block-size=1 index-block-size=1 table-format=(Pebble,v5)
235+
build block-size=1 index-block-size=1 table-format=Pebble,v5
236236
a.SET.1:a
237237
d.SET.2:d
238238
f.SET.3:f
@@ -262,7 +262,7 @@ props:
262262
rocksdb.num.data.blocks: 1
263263
rocksdb.compression: Snappy
264264

265-
build table-format=(Pebble,v4)
265+
build table-format=Pebble,v4
266266
a.SET.1:a
267267
b.SET.2:b
268268
c.SET.3:c
@@ -288,7 +288,7 @@ props:
288288

289289
# Repeat the above with (Pebble,v5).
290290

291-
build table-format=(Pebble,v5)
291+
build table-format=Pebble,v5
292292
a.SET.1:a
293293
b.SET.2:b
294294
c.SET.3:c

‎sstable/testdata/writer‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ c#1,SET:c
253253
# Enabling leveldb format disables the creation of a two-level index
254254
# (the input data here mirrors the test case above).
255255

256-
build leveldb block-size=1 index-block-size=1
256+
build table-format=LevelDB block-size=1 index-block-size=1
257257
a.SET.1:a
258258
b.SET.1:b
259259
c.SET.1:c

‎sstable/testdata/writer_blob_value_handles‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
build table-format=(Pebble,v6)
1+
build table-format=Pebble,v6
22
a@2.SET.1:blobInlineHandle(0, blk1, 10, 100, 0x07)
33
b@5.SET.7:blobInlineHandle(0, blk2, 110, 200, 0x07)
44
b@4.DEL.3:

0 commit comments

Comments
 (0)