Skip to content

Commit 5c86dc7

Browse files
miparnisarimiparnisari
committed
fix: error message when cannot run 'datastore gc' or 'datastore repair'
1 parent b5be205 commit 5c86dc7

File tree

4 files changed

+168
-66
lines changed

4 files changed

+168
-66
lines changed

‎internal/datastore/crdb/crdb.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ func (cds *crdbDatastore) ReadWriteTx(
364364
// a deletion of expired relationships.
365365
//
366366
// A transaction is marked as such IF and only IF the operations in the transaction
367-
// consist solely of deletions, as in that scenario, we cannot be certain in the Watc
367+
// consist solely of deletions, as in that scenario, we cannot be certain in the Watch
368368
// changefeed that the transaction is not a deletion of expired relationships performed
369369
// by CRDB itself. This is also only necessary if both expiration and watch are enabled.
370370
metadata := config.Metadata.AsMap()

‎magefiles/test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func (t Test) All(ctx context.Context) {
2222
c := Testcons{}
2323
mg.CtxDeps(ctx, t.Unit, t.Integration, t.Steelthread, t.Image, t.Analyzers,
2424
ds.Crdb, ds.Postgres, ds.Spanner, ds.Mysql,
25-
c.Crdb, c.Spanner, c.Postgres, c.Mysql)
25+
c.Crdb, c.Postgres, c.Spanner, c.Mysql)
2626
}
2727

2828
// UnitCover Runs the unit tests and generates a coverage report
@@ -62,7 +62,7 @@ func (Test) Integration(ctx context.Context) error {
6262
return goTest(ctx, "./internal/services/integrationtesting/...", "-tags", "ci,docker", "-timeout", "15m")
6363
}
6464

65-
// Integration Run integration tests with cover
65+
// IntegrationCover Run integration tests with cover
6666
func (Test) IntegrationCover(ctx context.Context) error {
6767
mg.Deps(checkDocker)
6868
args := []string{"-tags", "ci,docker", "-timeout", "15m", "-count=1"}

‎pkg/cmd/datastore.go‎

Lines changed: 71 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -60,80 +60,88 @@ func NewGCDatastoreCommand(programName string, cfg *datastore.Config) *cobra.Com
6060
Long: "Executes garbage collection against the datastore. Deletes stale relationships, expired relationships, and stale transactions.",
6161
PreRunE: server.DefaultPreRunE(programName),
6262
RunE: termination.PublishError(func(cmd *cobra.Command, args []string) error {
63-
ctx := context.Background()
64-
65-
// Disable background GC and hedging.
66-
cfg.GCInterval = -1 * time.Hour
67-
cfg.RequestHedgingEnabled = false
68-
69-
ds, err := datastore.NewDatastore(ctx, cfg.ToOption())
70-
if err != nil {
71-
return fmt.Errorf("failed to create datastore: %w", err)
72-
}
73-
74-
gcds := dspkg.UnwrapAs[common.GarbageCollectableDatastore](ds)
75-
if gcds == nil {
76-
return fmt.Errorf("datastore of type %T does not support garbage collection", ds)
77-
}
78-
79-
log.Ctx(ctx).Info().
80-
Float64("gc_window_seconds", cfg.GCWindow.Seconds()).
81-
Float64("gc_max_operation_time_seconds", cfg.GCMaxOperationTime.Seconds()).
82-
Msg("Running garbage collection...")
83-
84-
err = common.RunGarbageCollection(gcds, cfg.GCWindow, cfg.GCMaxOperationTime)
85-
if err != nil {
86-
return err
87-
}
88-
89-
log.Ctx(ctx).Info().Msg("Garbage collection completed")
90-
return nil
63+
return executeGC(cfg)
9164
}),
9265
}
9366
}
9467

68+
func executeGC(cfg *datastore.Config) error {
69+
ctx := context.Background()
70+
71+
// Disable background GC and hedging.
72+
cfg.GCInterval = -1 * time.Hour
73+
cfg.RequestHedgingEnabled = false
74+
75+
ds, err := datastore.NewDatastore(ctx, cfg.ToOption())
76+
if err != nil {
77+
return fmt.Errorf("failed to create datastore: %w", err)
78+
}
79+
80+
gcds := dspkg.UnwrapAs[common.GarbageCollectableDatastore](ds)
81+
if gcds == nil {
82+
return fmt.Errorf("datastore of type '%s' does not support garbage collection", cfg.Engine)
83+
}
84+
85+
log.Ctx(ctx).Info().
86+
Float64("gc_window_seconds", cfg.GCWindow.Seconds()).
87+
Float64("gc_max_operation_time_seconds", cfg.GCMaxOperationTime.Seconds()).
88+
Msg("Running garbage collection...")
89+
90+
err = common.RunGarbageCollection(gcds, cfg.GCWindow, cfg.GCMaxOperationTime)
91+
if err != nil {
92+
return err
93+
}
94+
95+
log.Ctx(ctx).Info().Msg("Garbage collection completed")
96+
return nil
97+
}
98+
9599
func NewRepairDatastoreCommand(programName string, cfg *datastore.Config) *cobra.Command {
96100
return &cobra.Command{
97101
Use: "repair",
98102
Short: "executes datastore repair",
99103
Long: "Executes a repair operation for the datastore",
100104
PreRunE: server.DefaultPreRunE(programName),
101105
RunE: termination.PublishError(func(cmd *cobra.Command, args []string) error {
102-
ctx := context.Background()
103-
104-
// Disable background GC and hedging.
105-
cfg.GCInterval = -1 * time.Hour
106-
cfg.RequestHedgingEnabled = false
107-
108-
ds, err := datastore.NewDatastore(ctx, cfg.ToOption())
109-
if err != nil {
110-
return fmt.Errorf("failed to create datastore: %w", err)
111-
}
112-
113-
repairable := dspkg.UnwrapAs[dspkg.RepairableDatastore](ds)
114-
if repairable == nil {
115-
return fmt.Errorf("datastore of type %T does not support the repair operation", ds)
116-
}
117-
118-
if len(args) == 0 {
119-
fmt.Println()
120-
fmt.Println("Available repair operations:")
121-
for _, op := range repairable.RepairOperations() {
122-
fmt.Printf("\t%s: %s\n", op.Name, op.Description)
123-
}
124-
return nil
125-
}
126-
127-
operationName := args[0]
128-
129-
log.Ctx(ctx).Info().Msg("Running repair...")
130-
err = repairable.Repair(ctx, operationName, true)
131-
if err != nil {
132-
return err
133-
}
134-
135-
log.Ctx(ctx).Info().Msg("Datastore repair completed")
136-
return nil
106+
return executeRepair(cfg, args)
137107
}),
138108
}
139109
}
110+
111+
func executeRepair(cfg *datastore.Config, args []string) error {
112+
ctx := context.Background()
113+
114+
// Disable background GC and hedging.
115+
cfg.GCInterval = -1 * time.Hour
116+
cfg.RequestHedgingEnabled = false
117+
118+
ds, err := datastore.NewDatastore(ctx, cfg.ToOption())
119+
if err != nil {
120+
return fmt.Errorf("failed to create datastore: %w", err)
121+
}
122+
123+
repairable := dspkg.UnwrapAs[dspkg.RepairableDatastore](ds)
124+
if repairable == nil {
125+
return fmt.Errorf("datastore of type '%s' does not support the repair operation", cfg.Engine)
126+
}
127+
128+
if len(args) == 0 {
129+
fmt.Println()
130+
fmt.Println("Available repair operations:")
131+
for _, op := range repairable.RepairOperations() {
132+
fmt.Printf("\t%s: %s\n", op.Name, op.Description)
133+
}
134+
return nil
135+
}
136+
137+
operationName := args[0]
138+
139+
log.Ctx(ctx).Info().Msg("Running repair...")
140+
err = repairable.Repair(ctx, operationName, true)
141+
if err != nil {
142+
return err
143+
}
144+
145+
log.Ctx(ctx).Info().Msg("Datastore repair completed")
146+
return nil
147+
}

‎pkg/cmd/datastore_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
//go:build ci && docker
2+
// +build ci,docker
3+
4+
package cmd
5+
6+
import (
7+
"testing"
8+
9+
datastoreTest "github.com/authzed/spicedb/internal/testserver/datastore"
10+
"github.com/authzed/spicedb/pkg/cmd/datastore"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestExecuteGC(t *testing.T) {
15+
t.Parallel()
16+
17+
tests := []struct {
18+
name string
19+
cfgBuilder func(t *testing.T) *datastore.Config
20+
expectedError string
21+
}{
22+
{
23+
name: "cockroachdb does not support garbage collection",
24+
cfgBuilder: func(t *testing.T) *datastore.Config {
25+
cfg := datastore.DefaultDatastoreConfig()
26+
cfg.Engine = "cockroachdb"
27+
runningDatastore := datastoreTest.RunDatastoreEngine(t, cfg.Engine)
28+
db := runningDatastore.NewDatabase(t)
29+
cfg.URI = db
30+
return cfg
31+
},
32+
expectedError: "datastore of type 'cockroachdb' does not support garbage collection",
33+
},
34+
}
35+
36+
for _, tt := range tests {
37+
t.Run(tt.name, func(t *testing.T) {
38+
t.Parallel()
39+
40+
cfg := tt.cfgBuilder(t)
41+
err := executeGC(cfg)
42+
require.ErrorContains(t, err, tt.expectedError)
43+
})
44+
}
45+
}
46+
47+
func TestExecuteRepair(t *testing.T) {
48+
t.Parallel()
49+
50+
tests := []struct {
51+
name string
52+
cfgBuilder func(t *testing.T) *datastore.Config
53+
expectedError string
54+
}{
55+
{
56+
name: "cockroachdb does not support repair",
57+
cfgBuilder: func(t *testing.T) *datastore.Config {
58+
cfg := datastore.DefaultDatastoreConfig()
59+
cfg.Engine = "cockroachdb"
60+
runningDatastore := datastoreTest.RunDatastoreEngine(t, cfg.Engine)
61+
db := runningDatastore.NewDatabase(t)
62+
cfg.URI = db
63+
return cfg
64+
},
65+
expectedError: "datastore of type 'cockroachdb' does not support the repair operation",
66+
},
67+
{
68+
name: "postgres supports repair",
69+
cfgBuilder: func(t *testing.T) *datastore.Config {
70+
cfg := datastore.DefaultDatastoreConfig()
71+
cfg.Engine = "postgres"
72+
runningDatastore := datastoreTest.RunDatastoreEngine(t, cfg.Engine)
73+
db := runningDatastore.NewDatabase(t)
74+
cfg.URI = db
75+
return cfg
76+
},
77+
expectedError: "",
78+
},
79+
}
80+
81+
for _, tt := range tests {
82+
t.Run(tt.name, func(t *testing.T) {
83+
t.Parallel()
84+
85+
cfg := tt.cfgBuilder(t)
86+
err := executeRepair(cfg, []string{})
87+
if tt.expectedError == "" {
88+
require.NoError(t, err)
89+
return
90+
}
91+
require.ErrorContains(t, err, tt.expectedError)
92+
})
93+
}
94+
}

0 commit comments

Comments
 (0)