Skip to content

Commit

Permalink
Revert "Add follower read delay option"
Browse files Browse the repository at this point in the history
  • Loading branch information
ecordell committed Dec 6, 2021
1 parent 53ab33c commit b7749af
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 66 deletions.
2 changes: 0 additions & 2 deletions cmd/spicedb/serve.go
Expand Up @@ -86,7 +86,6 @@ func registerServeCmd(rootCmd *cobra.Command) {
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")
serveCmd.Flags().Duration("datastore-follower-read-delay-duration", 0*time.Second, "amount of time to use as a delay 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")
Expand Down Expand Up @@ -165,7 +164,6 @@ func serveRun(cmd *cobra.Command, args []string) {
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),
Expand Down
7 changes: 1 addition & 6 deletions internal/datastore/crdb/crdb.go
Expand Up @@ -124,16 +124,13 @@ func NewCRDBDatastore(url string, options ...Option) (datastore.Datastore, error
maxRevisionStaleness := time.Duration(float64(config.revisionQuantization.Nanoseconds())*
config.maxRevisionStalenessPercent) * time.Nanosecond

followerReadDelayNanos := config.followerReadDelay.Nanoseconds()

return &crdbDatastore{
dburl: url,
conn: conn,
watchBufferLength: config.watchBufferLength,
quantizationNanos: config.revisionQuantization.Nanoseconds(),
maxRevisionStaleness: maxRevisionStaleness,
gcWindowNanos: gcWindowNanos,
followerReadDelayNanos: followerReadDelayNanos,
splitAtEstimatedQuerySize: config.splitAtEstimatedQuerySize,
execute: executeWithMaxRetries(config.maxRetries),
overlapKeyer: keyer,
Expand All @@ -147,7 +144,6 @@ type crdbDatastore struct {
quantizationNanos int64
maxRevisionStaleness time.Duration
gcWindowNanos int64
followerReadDelayNanos int64
splitAtEstimatedQuerySize units.Base2Bytes
execute executeTxRetryFunc
overlapKeyer overlapKeyer
Expand Down Expand Up @@ -199,8 +195,7 @@ func (cds *crdbDatastore) Revision(ctx context.Context) (datastore.Revision, err
}

// Round the revision down to the nearest quantization
// Apply a delay to enable historial reads
crdbNow := nowHLC.IntPart() - cds.followerReadDelayNanos
crdbNow := nowHLC.IntPart()
quantized := crdbNow
if cds.quantizationNanos > 0 {
quantized -= (crdbNow % cds.quantizationNanos)
Expand Down
50 changes: 4 additions & 46 deletions internal/datastore/crdb/crdb_test.go
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/jackc/pgx/v4"
"github.com/ory/dockertest/v3"
"github.com/stretchr/testify/require"

"github.com/authzed/spicedb/internal/datastore"
"github.com/authzed/spicedb/internal/datastore/crdb/migrations"
Expand All @@ -22,11 +21,9 @@ import (
)

type sqlTest struct {
conn *pgx.Conn
port string
creds string
followerReadDelay time.Duration

conn *pgx.Conn
port string
creds string
cleanup func()
}

Expand Down Expand Up @@ -71,7 +68,6 @@ func (st sqlTest) New(revisionFuzzingTimedelta, gcWindow time.Duration, watchBuf
GCWindow(gcWindow),
RevisionQuantization(revisionFuzzingTimedelta),
WatchBufferLength(watchBufferLength),
FollowerReadDelay(st.followerReadDelay),
)
}

Expand All @@ -82,44 +78,6 @@ func TestCRDBDatastore(t *testing.T) {
test.All(t, tester)
}

func TestCRDBDatastoreWithFollowerReads(t *testing.T) {
followerReadDelay := time.Duration(4.8 * float64(time.Second))
gcWindow := 100 * time.Second

quantizationDurations := []time.Duration{
0 * time.Second,
100 * time.Millisecond,
}
for _, quantization := range quantizationDurations {
t.Run(fmt.Sprintf("followReadDelayWithQuantization%s", quantization), func(t *testing.T) {
require := require.New(t)

tester := newTester(crdbContainer, "root:fake", 26257)
tester.followerReadDelay = followerReadDelay

ds, err := tester.New(quantization, gcWindow, 1)
require.NoError(err)

ctx := context.Background()
ok, err := ds.IsReady(ctx)
require.NoError(err)
require.True(ok)

// Revisions should be at least the follower read delay amount in the past
for start := time.Now(); time.Since(start) < 50*time.Millisecond; {
testRevision, err := ds.Revision(ctx)
nowRevision, err := ds.SyncRevision(ctx)
require.NoError(err)

diff := nowRevision.IntPart() - testRevision.IntPart()
require.True(diff > followerReadDelay.Nanoseconds())
}
tester.cleanup()
ds.Close()
})
}
}

func newTester(containerOpts *dockertest.RunOptions, creds string, portNum uint16) *sqlTest {
pool, err := dockertest.NewPool("")
if err != nil {
Expand Down Expand Up @@ -151,5 +109,5 @@ func newTester(containerOpts *dockertest.RunOptions, creds string, portNum uint1
}
}

return &sqlTest{conn: conn, port: port, cleanup: cleanup, creds: creds, followerReadDelay: 0 * time.Second}
return &sqlTest{conn: conn, port: port, cleanup: cleanup, creds: creds}
}
12 changes: 0 additions & 12 deletions internal/datastore/crdb/options.go
Expand Up @@ -17,7 +17,6 @@ type crdbOptions struct {

watchBufferLength uint16
revisionQuantization time.Duration
followerReadDelay time.Duration
maxRevisionStalenessPercent float64
gcWindow time.Duration
maxRetries int
Expand All @@ -34,7 +33,6 @@ const (
overlapStrategyInsecure = "insecure"

defaultRevisionQuantization = 5 * time.Second
defaultFollowerReadDelay = 0 * time.Second
defaultMaxRevisionStalenessPercent = 0.1
defaultWatchBufferLength = 128

Expand All @@ -52,7 +50,6 @@ func generateConfig(options []Option) (crdbOptions, error) {
gcWindow: 24 * time.Hour,
watchBufferLength: defaultWatchBufferLength,
revisionQuantization: defaultRevisionQuantization,
followerReadDelay: defaultFollowerReadDelay,
maxRevisionStalenessPercent: defaultMaxRevisionStalenessPercent,
splitAtEstimatedQuerySize: common.DefaultSplitAtEstimatedQuerySize,
maxRetries: defaultMaxRetries,
Expand Down Expand Up @@ -146,15 +143,6 @@ func RevisionQuantization(bucketSize time.Duration) Option {
}
}

// FollowerReadDelay is the time delay to apply to enable historial reads.
//
// This value defaults to 0 seconds.
func FollowerReadDelay(delay time.Duration) Option {
return func(po *crdbOptions) {
po.followerReadDelay = delay
}
}

// MaxRevisionStalenessPercent is the amount of time, expressed as a percentage of
// the revision quantization window, that a previously computed rounded revision
// can still be advertised after the next rounded revision would otherwise be ready.
Expand Down

0 comments on commit b7749af

Please sign in to comment.