Skip to content

Commit

Permalink
Remove sleep in stats test unless needed
Browse files Browse the repository at this point in the history
The sleep was originally added for CRDB, which requires time for follower reads to catch up. However, the other datastores don't need this, which meant an extra 5s delay in most datastore tests
  • Loading branch information
josephschorr committed Jan 10, 2025
1 parent 7b9f388 commit fec41ee
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions pkg/datastore/test/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package test

import (
"context"
"strings"
"testing"
"time"

Expand All @@ -10,7 +11,7 @@ import (
"github.com/authzed/spicedb/internal/testfixtures"
)

const statsRetryCount = 3
const statsRetryCount = 10

func StatsTest(t *testing.T, tester DatastoreTester) {
ctx := context.Background()
Expand All @@ -21,23 +22,27 @@ func StatsTest(t *testing.T, tester DatastoreTester) {

ds, _ = testfixtures.StandardDatastoreWithData(ds, require)

// stats use follower reads, need to wait a bit so that the base tables
// have a chance to be follower-read
time.Sleep(5 * time.Second)

for retryCount := statsRetryCount; retryCount >= 0; retryCount-- {
stats, err := ds.Statistics(ctx)

// If the error contains code 3D000, the *database* is not yet available, which
// can happen due to using follower reads in CockroachDB. We should retry in this case.
if err != nil && strings.Contains(err.Error(), "3D000") {
time.Sleep(5 * time.Second)
continue
}

require.NoError(err)

require.Len(stats.UniqueID, 36, "unique ID must be a valid UUID")
require.Len(stats.ObjectTypeStatistics, 3, "must report object stats")

if stats.EstimatedRelationshipCount == uint64(0) && retryCount > 0 {
// Sleep for a bit to get the stats table to update.
time.Sleep(500 * time.Millisecond)
continue
}

require.Len(stats.ObjectTypeStatistics, 3, "must report object stats")
require.Greater(stats.EstimatedRelationshipCount, uint64(0), "must report some relationships")

newStats, err := ds.Statistics(ctx)
Expand Down

0 comments on commit fec41ee

Please sign in to comment.