Skip to content

Fix flaky tests in compactor #2018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 24, 2020
Merged

Fix flaky tests in compactor #2018

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions pkg/compactor/compactor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,12 @@ func TestConfig_ShouldSupportCliFlags(t *testing.T) {
func TestCompactor_ShouldDoNothingOnNoUserBlocks(t *testing.T) {
t.Parallel()

c, bucketClient, _, logs, registry := prepare(t)

// No user blocks stored in the bucket.
bucketClient := &cortex_tsdb.BucketClientMock{}
bucketClient.MockIter("", []string{}, nil)

c, _, logs, registry := prepare(t, bucketClient)

// Wait until a run has completed.
cortex_testutil.Poll(t, time.Second, 1.0, func() interface{} {
return prom_testutil.ToFloat64(c.compactionRunsCompleted)
Expand Down Expand Up @@ -104,11 +105,12 @@ func TestCompactor_ShouldDoNothingOnNoUserBlocks(t *testing.T) {
func TestCompactor_ShouldRetryOnFailureWhileDiscoveringUsersFromBucket(t *testing.T) {
t.Parallel()

c, bucketClient, _, logs, registry := prepare(t)

// Fail to iterate over the bucket while discovering users.
bucketClient := &cortex_tsdb.BucketClientMock{}
bucketClient.MockIter("", nil, errors.New("failed to iterate the bucket"))

c, _, logs, registry := prepare(t, bucketClient)

// Wait until all retry attempts have completed.
cortex_testutil.Poll(t, time.Second, 1.0, func() interface{} {
return prom_testutil.ToFloat64(c.compactionRunsFailed)
Expand Down Expand Up @@ -146,15 +148,16 @@ func TestCompactor_ShouldRetryOnFailureWhileDiscoveringUsersFromBucket(t *testin
func TestCompactor_ShouldIterateOverUsersAndRunCompaction(t *testing.T) {
t.Parallel()

c, bucketClient, tsdbCompactor, logs, registry := prepare(t)

// Mock the bucket to contain two users, each one with one block.
bucketClient := &cortex_tsdb.BucketClientMock{}
bucketClient.MockIter("", []string{"user-1", "user-2"}, nil)
bucketClient.MockIter("user-1/", []string{"user-1/01DTVP434PA9VFXSW2JKB3392D"}, nil)
bucketClient.MockIter("user-2/", []string{"user-2/01DTW0ZCPDDNV4BV83Q2SV4QAZ"}, nil)
bucketClient.MockGet("user-1/01DTVP434PA9VFXSW2JKB3392D/meta.json", mockBlockMetaJSON("01DTVP434PA9VFXSW2JKB3392D"), nil)
bucketClient.MockGet("user-2/01DTW0ZCPDDNV4BV83Q2SV4QAZ/meta.json", mockBlockMetaJSON("01DTW0ZCPDDNV4BV83Q2SV4QAZ"), nil)

c, tsdbCompactor, logs, registry := prepare(t, bucketClient)

// Mock the compactor as if there's no compaction to do,
// in order to simplify tests (all in all, we just want to
// test our logic and not TSDB compactor which we expect to
Expand Down Expand Up @@ -203,14 +206,13 @@ func TestCompactor_ShouldIterateOverUsersAndRunCompaction(t *testing.T) {
`)))
}

func prepare(t *testing.T) (*Compactor, *cortex_tsdb.BucketClientMock, *tsdbCompactorMock, *bytes.Buffer, prometheus.Gatherer) {
func prepare(t *testing.T, bucketClient *cortex_tsdb.BucketClientMock) (*Compactor, *tsdbCompactorMock, *bytes.Buffer, prometheus.Gatherer) {
compactorCfg := Config{}
storageCfg := cortex_tsdb.Config{}
flagext.DefaultValues(&compactorCfg, &storageCfg)
compactorCfg.retryMinBackoff = 0
compactorCfg.retryMaxBackoff = 0

bucketClient := &cortex_tsdb.BucketClientMock{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me but I am trying to understand how this was causing tests to fail intermittently since it was still created before calling newCompactor down below. Anything I am missing to see here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was created before newCompactor() but mocked later (after the prepare() exits).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, my bad. Changes look more relevant for this block

Thanks!

tsdbCompactor := &tsdbCompactorMock{}
logs := &bytes.Buffer{}
logger := log.NewLogfmtLogger(logs)
Expand All @@ -220,7 +222,7 @@ func prepare(t *testing.T) (*Compactor, *cortex_tsdb.BucketClientMock, *tsdbComp
c, err := newCompactor(ctx, cancelCtx, compactorCfg, storageCfg, bucketClient, tsdbCompactor, logger, registry)
require.NoError(t, err)

return c, bucketClient, tsdbCompactor, logs, registry
return c, tsdbCompactor, logs, registry
}

type tsdbCompactorMock struct {
Expand Down