Merge pull request #317 from ecordell/ds-builder

Simplify datastore construction

cmd/spicedb/serve.go

@@ -8,7 +8,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/alecthomas/units"
 	"github.com/fatih/color"
 	grpcauth "github.com/grpc-ecosystem/go-grpc-middleware/auth"
 	grpczerolog "github.com/grpc-ecosystem/go-grpc-middleware/providers/zerolog/v2"
@@ -23,30 +22,29 @@ import (
 
 	"github.com/authzed/spicedb/internal/auth"
 	"github.com/authzed/spicedb/internal/dashboard"
-	"github.com/authzed/spicedb/internal/datastore"
-	"github.com/authzed/spicedb/internal/datastore/common"
-	"github.com/authzed/spicedb/internal/datastore/crdb"
-	"github.com/authzed/spicedb/internal/datastore/memdb"
-	"github.com/authzed/spicedb/internal/datastore/postgres"
 	"github.com/authzed/spicedb/internal/datastore/proxy"
 	combineddispatch "github.com/authzed/spicedb/internal/dispatch/combined"
 	"github.com/authzed/spicedb/internal/gateway"
 	"github.com/authzed/spicedb/internal/middleware/servicespecific"
 	"github.com/authzed/spicedb/internal/namespace"
 	"github.com/authzed/spicedb/internal/services"
 	v1alpha1svc "github.com/authzed/spicedb/internal/services/v1alpha1"
+	cmdlib "github.com/authzed/spicedb/pkg/cmd"
 	logmw "github.com/authzed/spicedb/pkg/middleware/logging"
 	"github.com/authzed/spicedb/pkg/middleware/requestid"
 	"github.com/authzed/spicedb/pkg/validationfile"
 )
 
 func registerServeCmd(rootCmd *cobra.Command) {
+	var datastoreOptions cmdlib.DatastoreConfig
 	serveCmd := &cobra.Command{
 		Use:     "serve",
 		Short:   "serve the permissions database",
 		Long:    "A database that stores, computes, and validates application permissions",
 		PreRunE: defaultPreRunE,
-		Run:     serveRun,
+		Run: func(cmd *cobra.Command, args []string) {
+			serveRun(cmd, args, datastoreOptions)
+		},
 		Example: fmt.Sprintf(`	%s:
 		spicedb serve --grpc-preshared-key "somerandomkeyhere"
 
@@ -66,26 +64,10 @@ func registerServeCmd(rootCmd *cobra.Command) {
 	}
 
 	// Flags for the datastore
-	serveCmd.Flags().String("datastore-engine", "memory", `type of datastore to initialize ("memory", "postgres", "cockroachdb")`)
-	serveCmd.Flags().String("datastore-conn-uri", "", `connection string used by remote datastores (e.g. "postgres://postgres:password@localhost:5432/spicedb")`)
+	cmdlib.RegisterDatastoreFlags(serveCmd, &datastoreOptions)
 	serveCmd.Flags().Bool("datastore-readonly", false, "set the service to read-only mode")
-	serveCmd.Flags().Int("datastore-conn-max-open", 20, "number of concurrent connections open in a remote datastore's connection pool")
-	serveCmd.Flags().Int("datastore-conn-min-open", 10, "number of minimum concurrent connections open in a remote datastore's connection pool")
-	serveCmd.Flags().Duration("datastore-conn-max-lifetime", 30*time.Minute, "maximum amount of time a connection can live in a remote datastore's connection pool")
-	serveCmd.Flags().Duration("datastore-conn-max-idletime", 30*time.Minute, "maximum amount of time a connection can idle in a remote datastore's connection pool")
-	serveCmd.Flags().Duration("datastore-conn-healthcheck-interval", 30*time.Second, "time between a remote datastore's connection pool health checks")
-	serveCmd.Flags().Duration("datastore-gc-window", 24*time.Hour, "amount of time before revisions are garbage collected")
-	serveCmd.Flags().Duration("datastore-gc-interval", 3*time.Minute, "amount of time between passes of garbage collection (postgres driver only)")
-	serveCmd.Flags().Duration("datastore-gc-max-operation-time", 1*time.Minute, "maximum amount of time a garbage collection pass can operate before timing out (postgres driver only)")
-	serveCmd.Flags().Duration("datastore-revision-fuzzing-duration", 5*time.Second, "amount of time to advertize stale revisions")
-	// See crdb doc for info about follower reads and how it is configured: https://www.cockroachlabs.com/docs/stable/follower-reads.html
-	serveCmd.Flags().Duration("datastore-follower-read-delay-duration", 4_800*time.Millisecond, "amount of time to subtract from non-sync revision timestamps to ensure they are sufficiently in the past to enable follower reads (cockroach driver only)")
-	serveCmd.Flags().String("datastore-query-split-size", common.DefaultSplitAtEstimatedQuerySize.String(), "estimated number of bytes at which a query is split when using a remote datastore")
 	serveCmd.Flags().StringSlice("datastore-bootstrap-files", []string{}, "bootstrap data yaml files to load")
 	serveCmd.Flags().Bool("datastore-bootstrap-overwrite", false, "overwrite any existing data with bootstrap data")
-	serveCmd.Flags().Int("datastore-max-tx-retries", 50, "number of times a retriable transaction should be retried (cockroach driver only)")
-	serveCmd.Flags().String("datastore-tx-overlap-strategy", "static", `strategy to generate transaction overlap keys ("prefix", "static", "insecure") (cockroach driver only)`)
-	serveCmd.Flags().String("datastore-tx-overlap-key", "key", "static key to touch when writing to ensure transactions overlap (only used if --datastore-tx-overlap-strategy=static is set; cockroach driver only)")
 
 	serveCmd.Flags().Bool("datastore-request-hedging", true, "enable request hedging")
 	serveCmd.Flags().Duration("datastore-request-hedging-initial-slow-value", 10*time.Millisecond, "initial value to use for slow datastore requests, before statistics have been collected")
@@ -121,75 +103,15 @@ func registerServeCmd(rootCmd *cobra.Command) {
 	rootCmd.AddCommand(serveCmd)
 }
 
-func serveRun(cmd *cobra.Command, args []string) {
+func serveRun(cmd *cobra.Command, args []string, datastoreOpts cmdlib.DatastoreConfig) {
 	token := cobrautil.MustGetStringExpanded(cmd, "grpc-preshared-key")
 	if len(token) < 1 {
 		log.Fatal().Msg("a preshared key must be provided via --grpc-preshared-key to authenticate API requests")
 	}
 
-	datastoreEngine := cobrautil.MustGetStringExpanded(cmd, "datastore-engine")
-	datastoreURI := cobrautil.MustGetStringExpanded(cmd, "datastore-conn-uri")
-
-	revisionFuzzingTimedelta := cobrautil.MustGetDuration(cmd, "datastore-revision-fuzzing-duration")
-	gcWindow := cobrautil.MustGetDuration(cmd, "datastore-gc-window")
-	maxRetries := cobrautil.MustGetInt(cmd, "datastore-max-tx-retries")
-	overlapKey := cobrautil.MustGetStringExpanded(cmd, "datastore-tx-overlap-key")
-	overlapStrategy := cobrautil.MustGetStringExpanded(cmd, "datastore-tx-overlap-strategy")
-
-	splitQuerySize, err := units.ParseBase2Bytes(cobrautil.MustGetStringExpanded(cmd, "datastore-query-split-size"))
+	ds, err := cmdlib.NewDatastore(datastoreOpts.ToOption())
 	if err != nil {
-		log.Fatal().Err(err).Msg("failed to parse datastore-query-split-size")
-	}
-
-	var ds datastore.Datastore
-	if datastoreEngine == "memory" {
-		log.Info().Msg("using in-memory datastore")
-		log.Warn().Msg("in-memory datastore is not persistent and not feasible to run in a high availability fashion")
-		ds, err = memdb.NewMemdbDatastore(0, revisionFuzzingTimedelta, gcWindow, 0)
-		if err != nil {
-			log.Fatal().Err(err).Msg("failed to init datastore")
-		}
-	} else if datastoreEngine == "cockroachdb" {
-		log.Info().Msg("using cockroachdb datastore")
-		ds, err = crdb.NewCRDBDatastore(
-			datastoreURI,
-			crdb.ConnMaxIdleTime(cobrautil.MustGetDuration(cmd, "datastore-conn-max-idletime")),
-			crdb.ConnMaxLifetime(cobrautil.MustGetDuration(cmd, "datastore-conn-max-lifetime")),
-			crdb.MaxOpenConns(cobrautil.MustGetInt(cmd, "datastore-conn-max-open")),
-			crdb.MinOpenConns(cobrautil.MustGetInt(cmd, "datastore-conn-min-open")),
-			crdb.RevisionQuantization(revisionFuzzingTimedelta),
-			crdb.FollowerReadDelay(cobrautil.MustGetDuration(cmd, "datastore-follower-read-delay-duration")),
-			crdb.GCWindow(gcWindow),
-			crdb.MaxRetries(maxRetries),
-			crdb.SplitAtEstimatedQuerySize(splitQuerySize),
-			crdb.OverlapKey(overlapKey),
-			crdb.OverlapStrategy(overlapStrategy),
-		)
-		if err != nil {
-			log.Fatal().Err(err).Msg("failed to init datastore")
-		}
-	} else if datastoreEngine == "postgres" {
-		log.Info().Msg("using postgres datastore")
-		ds, err = postgres.NewPostgresDatastore(
-			datastoreURI,
-			postgres.ConnMaxIdleTime(cobrautil.MustGetDuration(cmd, "datastore-conn-max-idletime")),
-			postgres.ConnMaxLifetime(cobrautil.MustGetDuration(cmd, "datastore-conn-max-lifetime")),
-			postgres.HealthCheckPeriod(cobrautil.MustGetDuration(cmd, "datastore-conn-healthcheck-interval")),
-			postgres.MaxOpenConns(cobrautil.MustGetInt(cmd, "datastore-conn-max-open")),
-			postgres.MinOpenConns(cobrautil.MustGetInt(cmd, "datastore-conn-min-open")),
-			postgres.RevisionFuzzingTimedelta(revisionFuzzingTimedelta),
-			postgres.GCInterval(cobrautil.MustGetDuration(cmd, "datastore-gc-interval")),
-			postgres.GCMaxOperationTime(cobrautil.MustGetDuration(cmd, "datastore-gc-max-operation-time")),
-			postgres.GCWindow(gcWindow),
-			postgres.EnablePrometheusStats(),
-			postgres.EnableTracing(),
-			postgres.SplitAtEstimatedQuerySize(splitQuerySize),
-		)
-		if err != nil {
-			log.Fatal().Err(err).Msg("failed to init datastore")
-		}
-	} else {
-		log.Fatal().Str("datastore-engine", datastoreEngine).Msg("unknown datastore engine type")
+		log.Fatal().Err(err).Msg("failed to init datastore")
 	}
 
 	bootstrapFilePaths := cobrautil.MustGetStringSlice(cmd, "datastore-bootstrap-files")
@@ -349,7 +271,7 @@ func serveRun(cmd *cobra.Command, args []string) {
 	dashboardSrv.Handler = dashboard.NewHandler(
 		cobrautil.MustGetStringExpanded(cmd, "grpc-addr"),
 		cobrautil.MustGetStringExpanded(cmd, "grpc-tls-cert-path") != "" && cobrautil.MustGetStringExpanded(cmd, "grpc-tls-key-path") != "",
-		datastoreEngine,
+		datastoreOpts.Engine,
 		ds,
 	)
 	go func() {

e2e/newenemy/newenemy_test.go

@@ -139,7 +139,9 @@ func TestNoNewEnemy(t *testing.T) {
 
 	samplestddev := stddev / math.Sqrt(float64(sampleSize))
 	// how many iterations do we need to get > 3sigma from the mean?
-	iterations := int(math.Ceil(3*stddev*samplestddev + mean))
+	// cap max_iterations to control test runtime
+	const max_iterations = 100
+	iterations := int(math.Min(max_iterations, math.Ceil(3*stddev*samplestddev+mean)))
 
 	t.Logf("check spicedb is protected after %d attempts", iterations)
 	// *6 to cover the worst case where all requests are handled by the slow node

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/containerd/continuity v0.2.1 // indirect
 	github.com/dgraph-io/ristretto v0.1.0
 	github.com/docker/docker v20.10.9+incompatible // indirect
+	github.com/ecordell/optgen v0.0.4
 	github.com/emirpasic/gods v1.12.0
 	github.com/envoyproxy/protoc-gen-validate v0.6.2
 	github.com/fatih/color v1.13.0

go.sum

@@ -136,6 +136,8 @@ github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7Do
 github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4=
+github.com/dave/jennifer v1.4.0 h1:tNJFJmLDVTLu+v05mVZ88RINa3vQqnyyWkTKWYz0CwE=
+github.com/dave/jennifer v1.4.0/go.mod h1:fIb+770HOpJ2fmN9EPPKOqm1vMGhB+TwXKMZhrIygKg=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -154,6 +156,8 @@ github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw
 github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
 github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
+github.com/ecordell/optgen v0.0.4 h1:0ytRxj6f/LBl7laGDxiUjjR+ioAqr4WMLpS84D8AD10=
+github.com/ecordell/optgen v0.0.4/go.mod h1:bAPkLVWcBlTX5EkXW0UTPRj3+yjq2I6VLgH8OasuQEM=
 github.com/emirpasic/gods v1.12.0 h1:QAUIPSaCu4G+POclxeqb3F+WPpdKqFGlw36+yOzGlrg=
 github.com/emirpasic/gods v1.12.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
@@ -767,6 +771,7 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.5.0 h1:UG21uOlmZabA4fW5i7ZX6bjw1xELEGg/ZLgZq9auk/Q=
 golang.org/x/mod v0.5.0/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -999,6 +1004,7 @@ golang.org/x/tools v0.0.0-20200515010526-7d3b6ebf133d/go.mod h1:EkVYQZoAsY45+roY
 golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200618134242-20370b0cb4b2/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200717024301-6ddee64345a6/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
 golang.org/x/tools v0.0.0-20200729194436-6467de6f59a7/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
 golang.org/x/tools v0.0.0-20200804011535-6c149bb5ef0d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
 golang.org/x/tools v0.0.0-20200825202427-b303f430e36d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=

internal/datastore/datastore.go

@@ -80,7 +80,7 @@ type GraphDatastore interface {
 	// from a subject relation onward from the datastore.
 	ReverseQueryTuplesFromSubjectRelation(subjectNamespace, subjectRelation string, revision Revision) ReverseTupleQuery
 
-	// ReverseQueryTuplesFromSubjectNamespace creates a builder for reading
+	// ReverseQueryTuplesFromSubjectNamespace creates a builder for reading
 	// tuples from a subject namespace onward from the datastore.
 	ReverseQueryTuplesFromSubjectNamespace(subjectNamespace string, revision Revision) ReverseTupleQuery