Skip to content

Commit 4744a80

Browse files
RaduBerindeRaduBerinde
committed
storageconfig: move StoreSpec
Note that the command-line (pflag) adapters are left out of storageconfig, as those will in time be deprecated. Epic: none Release note: None
1 parent c5a115d commit 4744a80

File tree

17 files changed

+78
-96
lines changed

17 files changed

+78
-96
lines changed

‎pkg/acceptance/cluster/dockercluster.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ func (l *DockerCluster) startNode(ctx context.Context, node *testNode, singleNod
487487
Path: store.dir,
488488
Size: storageconfig.Size{Bytes: int64(store.config.MaxRanges) * maxRangeBytes},
489489
}
490-
cmd = append(cmd, fmt.Sprintf("--store=%s", storeSpec))
490+
cmd = append(cmd, fmt.Sprintf("--store=%s", base.StoreSpecCmdLineString(storeSpec)))
491491
}
492492
// Append --join flag for all nodes.
493493
if !singleNode {

‎pkg/base/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ go_test(
6363
data = glob(["testdata/**"]),
6464
embed = [":base"],
6565
deps = [
66-
"//pkg/roachpb",
6766
"//pkg/security/securityassets",
6867
"//pkg/security/securitytest",
6968
"//pkg/storage/storageconfig",

‎pkg/base/store_spec.go‎

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"unicode"
1616

1717
"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
18-
"github.com/cockroachdb/cockroach/pkg/roachpb"
1918
"github.com/cockroachdb/cockroach/pkg/storage/storageconfig"
2019
"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
2120
"github.com/cockroachdb/errors"
@@ -85,31 +84,10 @@ func parseStoreProvisionedRate(
8584

8685
// StoreSpec contains the details that can be specified in the cli pertaining
8786
// to the --store flag.
88-
type StoreSpec struct {
89-
Path string
90-
Size storageconfig.Size
91-
BallastSize *storageconfig.Size
92-
InMemory bool
93-
Attributes roachpb.Attributes
94-
// StickyVFSID is a unique identifier associated with a given store which
95-
// will preserve the in-memory virtual file system (VFS) even after the
96-
// storage engine has been closed. This only applies to in-memory storage
97-
// engine.
98-
StickyVFSID string
99-
// PebbleOptions contains Pebble-specific options in the same format as a
100-
// Pebble OPTIONS file. For example:
101-
// [Options]
102-
// delete_range_flush_delay=2s
103-
// flush_split_bytes=4096
104-
PebbleOptions string
105-
// EncryptionOptions is set if encryption is enabled.
106-
EncryptionOptions *storageconfig.EncryptionOptions
107-
// ProvisionedRate is optional.
108-
ProvisionedRate storageconfig.ProvisionedRate
109-
}
87+
type StoreSpec = storageconfig.Store
11088

111-
// String returns a fully parsable version of the store spec.
112-
func (ss StoreSpec) String() string {
89+
// StoreSpecCmdLineString returns a fully parsable version of the store spec.
90+
func StoreSpecCmdLineString(ss storageconfig.Store) string {
11391
// TODO(jackson): Implement redact.SafeFormatter
11492
var buffer bytes.Buffer
11593
if len(ss.Path) != 0 {
@@ -132,9 +110,9 @@ func (ss StoreSpec) String() string {
132110
fmt.Fprintf(&buffer, "ballast-size=%s%%,", humanize.Ftoa(ss.BallastSize.Percent))
133111
}
134112
}
135-
if len(ss.Attributes.Attrs) > 0 {
113+
if len(ss.Attributes) > 0 {
136114
fmt.Fprint(&buffer, "attrs=")
137-
for i, attr := range ss.Attributes.Attrs {
115+
for i, attr := range ss.Attributes {
138116
if i != 0 {
139117
fmt.Fprint(&buffer, ":")
140118
}
@@ -159,11 +137,6 @@ func (ss StoreSpec) String() string {
159137
return buffer.String()
160138
}
161139

162-
// IsEncrypted returns whether the StoreSpec has encryption enabled.
163-
func (ss StoreSpec) IsEncrypted() bool {
164-
return ss.EncryptionOptions != nil
165-
}
166-
167140
// NewStoreSpec parses the string passed into a --store flag and returns a
168141
// StoreSpec if it is correctly parsed.
169142
// There are five possible fields that can be passed in, comma separated:
@@ -251,9 +224,9 @@ func NewStoreSpec(value string) (StoreSpec, error) {
251224
attrMap[attribute] = struct{}{}
252225
}
253226
for attribute := range attrMap {
254-
ss.Attributes.Attrs = append(ss.Attributes.Attrs, attribute)
227+
ss.Attributes = append(ss.Attributes, attribute)
255228
}
256-
sort.Strings(ss.Attributes.Attrs)
229+
sort.Strings(ss.Attributes)
257230
case "type":
258231
if value == "mem" {
259232
ss.InMemory = true
@@ -338,7 +311,7 @@ var _ pflag.Value = &StoreSpecList{}
338311
func (ssl StoreSpecList) String() string {
339312
var buffer bytes.Buffer
340313
for _, ss := range ssl.Specs {
341-
fmt.Fprintf(&buffer, "--%s=%s ", cliflags.Store.Name, ss)
314+
fmt.Fprintf(&buffer, "--%s=%s ", cliflags.Store.Name, StoreSpecCmdLineString(ss))
342315
}
343316
// Trim the extra space from the end if it exists.
344317
if l := buffer.Len(); l > 0 {
@@ -383,7 +356,10 @@ func (ssl StoreSpecList) PriorCriticalAlertError() (err error) {
383356
err = errors.WithDetailf(err, "%v", newErr)
384357
}
385358
for _, ss := range ssl.Specs {
386-
path := ss.PreventedStartupFile()
359+
if ss.InMemory {
360+
continue
361+
}
362+
path := PreventedStartupFile(filepath.Join(ss.Path, AuxiliaryDir))
387363
if path == "" {
388364
continue
389365
}
@@ -399,16 +375,6 @@ func (ssl StoreSpecList) PriorCriticalAlertError() (err error) {
399375
return err
400376
}
401377

402-
// PreventedStartupFile returns the path to a file which, if it exists, should
403-
// prevent the server from starting up. Returns an empty string for in-memory
404-
// engines.
405-
func (ss StoreSpec) PreventedStartupFile() string {
406-
if ss.InMemory {
407-
return ""
408-
}
409-
return PreventedStartupFile(filepath.Join(ss.Path, AuxiliaryDir))
410-
}
411-
412378
// Type returns the underlying type in string form. This is part of pflag's
413379
// value interface.
414380
func (ssl *StoreSpecList) Type() string {

‎pkg/base/store_spec_test.go‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"testing"
1414

1515
"github.com/cockroachdb/cockroach/pkg/base"
16-
"github.com/cockroachdb/cockroach/pkg/roachpb"
1716
"github.com/cockroachdb/cockroach/pkg/storage/storageconfig"
1817
"github.com/cockroachdb/cockroach/pkg/testutils"
1918
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -69,23 +68,23 @@ target_file_size=2097152`
6968
// attributes
7069
{"path=/mnt/hda1,attrs=ssd", "", StoreSpec{
7170
Path: "/mnt/hda1",
72-
Attributes: roachpb.Attributes{Attrs: []string{"ssd"}},
71+
Attributes: []string{"ssd"},
7372
}},
7473
{"path=/mnt/hda1,attrs=ssd:hdd", "", StoreSpec{
7574
Path: "/mnt/hda1",
76-
Attributes: roachpb.Attributes{Attrs: []string{"hdd", "ssd"}},
75+
Attributes: []string{"hdd", "ssd"},
7776
}},
7877
{"path=/mnt/hda1,attrs=hdd:ssd", "", StoreSpec{
7978
Path: "/mnt/hda1",
80-
Attributes: roachpb.Attributes{Attrs: []string{"hdd", "ssd"}},
79+
Attributes: []string{"hdd", "ssd"},
8180
}},
8281
{"attrs=ssd:hdd,path=/mnt/hda1", "", StoreSpec{
8382
Path: "/mnt/hda1",
84-
Attributes: roachpb.Attributes{Attrs: []string{"hdd", "ssd"}},
83+
Attributes: []string{"hdd", "ssd"},
8584
}},
8685
{"attrs=hdd:ssd,path=/mnt/hda1,", "", StoreSpec{
8786
Path: "/mnt/hda1",
88-
Attributes: roachpb.Attributes{Attrs: []string{"hdd", "ssd"}},
87+
Attributes: []string{"hdd", "ssd"},
8988
}},
9089
{"attrs=hdd:ssd", "no path specified", StoreSpec{}},
9190
{"path=/mnt/hda1,attrs=", "no value specified for attrs", StoreSpec{}},
@@ -104,21 +103,21 @@ target_file_size=2097152`
104103
{"path=/mnt/hda1,size=50.5%", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 50.5}}},
105104
{"path=/mnt/hda1,size=100%", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 100}}},
106105
{"path=/mnt/hda1,size=1%", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 1}}},
107-
{"path=/mnt/hda1,size=0.999999%", "store size (0.999999%) must be between 1.000000% and 100.000000%", StoreSpec{}},
108-
{"path=/mnt/hda1,size=100.0001%", "store size (100.0001%) must be between 1.000000% and 100.000000%", StoreSpec{}},
106+
{"path=/mnt/hda1,size=0.999999%", "size (0.999999%) must be between 1% and 100%", StoreSpec{}},
107+
{"path=/mnt/hda1,size=100.0001%", "size (100.0001%) must be between 1% and 100%", StoreSpec{}},
109108
// 0.xxx
110109
{"path=/mnt/hda1,size=0.99", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 99}}},
111110
{"path=/mnt/hda1,size=0.5000000", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 50}}},
112111
{"path=/mnt/hda1,size=0.01", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 1}}},
113-
{"path=/mnt/hda1,size=0.009999", "store size (0.009999) must be between 1.000000% and 100.000000%", StoreSpec{}},
112+
{"path=/mnt/hda1,size=0.009999", "size (0.009999) must be between 1% and 100%", StoreSpec{}},
114113
// .xxx
115114
{"path=/mnt/hda1,size=.999", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 99.9}}},
116115
{"path=/mnt/hda1,size=.5000000", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 50}}},
117116
{"path=/mnt/hda1,size=.01", "", StoreSpec{Path: "/mnt/hda1", Size: SizeSpec{Percent: 1}}},
118-
{"path=/mnt/hda1,size=.009999", "store size (.009999) must be between 1.000000% and 100.000000%", StoreSpec{}},
117+
{"path=/mnt/hda1,size=.009999", "size (.009999) must be between 1% and 100%", StoreSpec{}},
119118
// errors
120-
{"path=/mnt/hda1,size=0", "store size (0) must be larger than 640 MiB", StoreSpec{}},
121-
{"path=/mnt/hda1,size=abc", "could not parse store size (abc): strconv.ParseFloat: parsing \"\": invalid syntax", StoreSpec{}},
119+
{"path=/mnt/hda1,size=0", "size (0) must be at least 640 MiB", StoreSpec{}},
120+
{"path=/mnt/hda1,size=abc", "could not parse size (abc): strconv.ParseFloat: parsing \"\": invalid syntax", StoreSpec{}},
122121
{"path=/mnt/hda1,size=", "no value specified for size", StoreSpec{}},
123122
{"size=20GiB,path=/mnt/hda1,size=20GiB", "size field was used twice in store definition", StoreSpec{}},
124123
{"size=123TB", "no path specified", StoreSpec{}},
@@ -127,7 +126,7 @@ target_file_size=2097152`
127126
{"path=/mnt/hda1,ballast-size=671088640", "", StoreSpec{Path: "/mnt/hda1", BallastSize: &SizeSpec{Bytes: 671088640}}},
128127
{"path=/mnt/hda1,ballast-size=20GB", "", StoreSpec{Path: "/mnt/hda1", BallastSize: &SizeSpec{Bytes: 20000000000}}},
129128
{"path=/mnt/hda1,ballast-size=1%", "", StoreSpec{Path: "/mnt/hda1", BallastSize: &SizeSpec{Percent: 1}}},
130-
{"path=/mnt/hda1,ballast-size=100.000%", "ballast size (100.000%) must be between 0.000000% and 50.000000%", StoreSpec{}},
129+
{"path=/mnt/hda1,ballast-size=100.000%", "ballast: size (100.000%) must be between 0% and 50%", StoreSpec{}},
131130
{"ballast-size=20GiB,path=/mnt/hda1,ballast-size=20GiB", "ballast-size field was used twice in store definition", StoreSpec{}},
132131

133132
// type
@@ -137,9 +136,9 @@ target_file_size=2097152`
137136
{"size=20GiB,type=mem,attrs=mem", "", StoreSpec{
138137
Size: SizeSpec{Bytes: 21474836480},
139138
InMemory: true,
140-
Attributes: roachpb.Attributes{Attrs: []string{"mem"}},
139+
Attributes: []string{"mem"},
141140
}},
142-
{"type=mem,size=20", "store size (20) must be larger than 640 MiB", StoreSpec{}},
141+
{"type=mem,size=20", "size (20) must be at least 640 MiB", StoreSpec{}},
143142
{"type=mem,size=", "no value specified for size", StoreSpec{}},
144143
{"type=mem,attrs=ssd", "size must be specified for an in memory store", StoreSpec{}},
145144
{"path=/mnt/hda1,type=mem", "path specified for in memory store", StoreSpec{}},
@@ -163,12 +162,12 @@ target_file_size=2097152`
163162
{"path=/mnt/hda1,attrs=hdd:ssd,size=20GiB", "", StoreSpec{
164163
Path: "/mnt/hda1",
165164
Size: SizeSpec{Bytes: 21474836480},
166-
Attributes: roachpb.Attributes{Attrs: []string{"hdd", "ssd"}},
165+
Attributes: []string{"hdd", "ssd"},
167166
}},
168167
{"type=mem,attrs=hdd:ssd,size=20GiB", "", StoreSpec{
169168
Size: SizeSpec{Bytes: 21474836480},
170169
InMemory: true,
171-
Attributes: roachpb.Attributes{Attrs: []string{"hdd", "ssd"}},
170+
Attributes: []string{"hdd", "ssd"},
172171
}},
173172

174173
// other error cases
@@ -202,14 +201,14 @@ target_file_size=2097152`
202201
}
203202

204203
// Now test String() to make sure the result can be parsed.
205-
storeSpecString := storeSpec.String()
204+
storeSpecString := base.StoreSpecCmdLineString(storeSpec)
206205
storeSpec2, err := base.NewStoreSpec(storeSpecString)
207206
if err != nil {
208207
t.Errorf("%d(%s): error parsing String() result: %s", i, testCase.value, err)
209208
continue
210209
}
211210
// Compare strings to deal with floats not matching exactly.
212-
if !reflect.DeepEqual(storeSpecString, storeSpec2.String()) {
211+
if !reflect.DeepEqual(storeSpecString, base.StoreSpecCmdLineString(storeSpec2)) {
213212
t.Errorf("%d(%s): actual doesn't match expected\nactual: %#+v\nexpected: %#+v", i, testCase.value,
214213
storeSpec, storeSpec2)
215214
}
@@ -248,7 +247,7 @@ func TestStoreSpecListPreventedStartupMessage(t *testing.T) {
248247
err := ssl.PriorCriticalAlertError()
249248
require.NoError(t, err)
250249

251-
require.NoError(t, os.WriteFile(ssl.Specs[2].PreventedStartupFile(), []byte("boom"), 0644))
250+
require.NoError(t, os.WriteFile(base.PreventedStartupFile(okAuxDir), []byte("boom"), 0644))
252251

253252
err = ssl.PriorCriticalAlertError()
254253
require.Error(t, err)

‎pkg/base/test_server_args.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -591,16 +591,14 @@ func InternalNonDefaultDecision(
591591
return baseArg
592592
}
593593

594-
var (
595-
// DefaultTestStoreSpec is just a single in memory store of 512 MiB
596-
// with no special attributes.
597-
DefaultTestStoreSpec = StoreSpec{
598-
InMemory: true,
599-
Size: storageconfig.Size{
600-
Bytes: 512 << 20,
601-
},
602-
}
603-
)
594+
// DefaultTestStoreSpec is just a single in memory store of 512 MiB
595+
// with no special attributes.
596+
var DefaultTestStoreSpec = storageconfig.Store{
597+
InMemory: true,
598+
Size: storageconfig.Size{
599+
Bytes: 512 << 20,
600+
},
601+
}
604602

605603
// DefaultTestTempStorageConfig is the associated temp storage for
606604
// DefaultTestStoreSpec that is in-memory.

‎pkg/ccl/serverccl/diagnosticsccl/reporter_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ func startReporterTest(
592592
}
593593

594594
storeSpec := base.DefaultTestStoreSpec
595-
storeSpec.Attributes = roachpb.Attributes{Attrs: []string{elemName}}
595+
storeSpec.Attributes = []string{elemName}
596596
rt.serverArgs = base.TestServerArgs{
597597
DefaultTestTenant: defaultTestTenant,
598598
StoreSpecs: []base.StoreSpec{

‎pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func TestDataDriven(t *testing.T) {
122122
SQLExecutor: sqlExecutorKnobs,
123123
},
124124
StoreSpecs: []base.StoreSpec{
125-
{InMemory: true, Attributes: roachpb.Attributes{Attrs: []string{attr}}},
125+
{InMemory: true, Attributes: []string{attr}},
126126
},
127127
}
128128
}

‎pkg/ccl/storageccl/engineccl/encrypted_fs_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,6 @@ func TestPebbleEncryption(t *testing.T) {
251251
ctx,
252252
base.StoreSpec{
253253
InMemory: true,
254-
Attributes: roachpb.Attributes{},
255254
Size: storageconfig.Size{Bytes: 512 << 20},
256255
EncryptionOptions: encOptions,
257256
StickyVFSID: stickyVFSID,
@@ -300,7 +299,6 @@ func TestPebbleEncryption(t *testing.T) {
300299
ctx,
301300
base.StoreSpec{
302301
InMemory: true,
303-
Attributes: roachpb.Attributes{},
304302
Size: storageconfig.Size{Bytes: 512 << 20},
305303
EncryptionOptions: encOptions,
306304
StickyVFSID: stickyVFSID,
@@ -388,7 +386,6 @@ func TestPebbleEncryption2(t *testing.T) {
388386
ctx,
389387
base.StoreSpec{
390388
InMemory: true,
391-
Attributes: roachpb.Attributes{},
392389
Size: storageconfig.Size{Bytes: 512 << 20},
393390
EncryptionOptions: encOptions,
394391
StickyVFSID: stickyVFSID,

‎pkg/cli/flags.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1515,7 +1515,7 @@ func newSizeFlagVal(spec *storageconfig.Size) *sizeFlagVal {
15151515
// pflag.Value interface.
15161516
func (sv *sizeFlagVal) String() string {
15171517
if sv.spec.Percent != 0 {
1518-
return string(humanize.Ftoa(sv.spec.Percent)) + "%"
1518+
return humanize.Ftoa(sv.spec.Percent) + "%"
15191519
}
15201520
return string(humanizeutil.IBytes(sv.spec.Bytes))
15211521
}

‎pkg/kv/kvserver/replicate_queue_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,19 +1951,19 @@ func TestTransferLeaseToLaggingNode(t *testing.T) {
19511951
0: {
19521952
ScanMaxIdleTime: time.Millisecond,
19531953
StoreSpecs: []base.StoreSpec{{
1954-
InMemory: true, Attributes: roachpb.Attributes{Attrs: []string{"n1"}},
1954+
InMemory: true, Attributes: []string{"n1"},
19551955
}},
19561956
},
19571957
1: {
19581958
ScanMaxIdleTime: time.Millisecond,
19591959
StoreSpecs: []base.StoreSpec{{
1960-
InMemory: true, Attributes: roachpb.Attributes{Attrs: []string{"n2"}},
1960+
InMemory: true, Attributes: []string{"n2"},
19611961
}},
19621962
},
19631963
2: {
19641964
ScanMaxIdleTime: time.Millisecond,
19651965
StoreSpecs: []base.StoreSpec{{
1966-
InMemory: true, Attributes: roachpb.Attributes{Attrs: []string{"n3"}},
1966+
InMemory: true, Attributes: []string{"n3"},
19671967
}},
19681968
},
19691969
},

0 commit comments

Comments
 (0)