From 66af3a7e604882cf6599e2eebcb55fc1469f3fc3 Mon Sep 17 00:00:00 2001 From: Ben Brooks Date: Fri, 24 Jan 2025 13:47:02 +0000 Subject: [PATCH 1/2] Reinstate skip on TestLowSequenceHandlingNoDuplicates and detail assertion failure for flake caused by issue #3506 --- db/change_cache_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/db/change_cache_test.go b/db/change_cache_test.go index 0cd1ad5dd1..66a74ae336 100644 --- a/db/change_cache_test.go +++ b/db/change_cache_test.go @@ -989,6 +989,9 @@ func TestChannelQueryCancellation(t *testing.T) { } func TestLowSequenceHandlingNoDuplicates(t *testing.T) { + // TODO: Disabled until https://github.com/couchbase/sync_gateway/issues/3056 is fixed. + t.Skip("WARNING: TEST DISABLED") + base.SetUpTestLogging(t, base.LevelDebug, base.KeyChanges, base.KeyCache) db, ctx := setupTestDBWithCacheOptions(t, shortWaitCache()) @@ -1048,6 +1051,8 @@ func TestLowSequenceHandlingNoDuplicates(t *testing.T) { err = appendFromFeed(&changes, feed, 2, base.DefaultWaitForSequence) assert.True(t, err == nil) assert.Len(t, changes, 6) + // TODO: Test flake: Issue #3056 causing two seq 3 entries and losing seq 4 since we only pull in two more feed items + // {Seq:2::3, ID:doc-3, Changes:[map[rev:1-a]]} {Seq:3, ID:doc-3, Changes:[map[rev:1-a]]} assert.True(t, verifyChangesSequencesIgnoreOrder(changes, []uint64{1, 2, 5, 6, 3, 4})) WriteDirect(t, collection, []string{"ABC"}, 7) From 3774b18eb963af2b76d80b8fcbcbbbba4dd6a9b1 Mon Sep 17 00:00:00 2001 From: Ben Brooks Date: Fri, 24 Jan 2025 15:38:28 +0000 Subject: [PATCH 2/2] Delete TestLowSequenceHandlingNoDuplicates - it's unreliably reproing behaviour that doesn't cause functional issues, and there's no easy performant way to prevent this case, in the event that late sequences share common channels. --- db/change_cache_test.go | 77 ----------------------------------------- 1 file changed, 77 deletions(-) diff --git a/db/change_cache_test.go b/db/change_cache_test.go index 66a74ae336..e6fe1de9a5 100644 --- a/db/change_cache_test.go +++ b/db/change_cache_test.go @@ -988,83 +988,6 @@ func TestChannelQueryCancellation(t *testing.T) { assert.Equal(t, initialQueryCount+1, finalQueryCount) } -func TestLowSequenceHandlingNoDuplicates(t *testing.T) { - // TODO: Disabled until https://github.com/couchbase/sync_gateway/issues/3056 is fixed. - t.Skip("WARNING: TEST DISABLED") - - base.SetUpTestLogging(t, base.LevelDebug, base.KeyChanges, base.KeyCache) - - db, ctx := setupTestDBWithCacheOptions(t, shortWaitCache()) - defer db.Close(ctx) - - // Create a user with access to channel ABC - authenticator := db.Authenticator(ctx) - assert.True(t, authenticator != nil, "db.Authenticator(db.Ctx) returned nil") - user, err := authenticator.NewUser("naomi", "letmein", channels.BaseSetOf(t, "ABC", "PBS", "NBC", "TBS")) - assert.NoError(t, err, fmt.Sprintf("Error creating new user: %v", err)) - require.NoError(t, authenticator.Save(user)) - - collection := GetSingleDatabaseCollection(t, db.DatabaseContext) - // Simulate seq 3 and 4 being delayed - write 1,2,5,6 - WriteDirect(t, collection, []string{"ABC", "NBC"}, 1) - WriteDirect(t, collection, []string{"ABC"}, 2) - WriteDirect(t, collection, []string{"ABC", "PBS"}, 5) - WriteDirect(t, collection, []string{"ABC", "PBS"}, 6) - - require.NoError(t, db.changeCache.waitForSequence(ctx, 6, base.DefaultWaitForSequence)) - db.user, err = authenticator.GetUser("naomi") - require.NoError(t, err) - - // Start changes feed - - dbCollection, ctx := GetSingleDatabaseCollectionWithUser(ctx, t, db) - var options ChangesOptions - options.Since = SequenceID{Seq: 0} - ctx, changesCtxCancel := context.WithCancel(ctx) - options.ChangesCtx = ctx - defer changesCtxCancel() - options.Continuous = true - options.Wait = true - feed, err := dbCollection.MultiChangesFeed(ctx, base.SetOf("*"), options) - assert.True(t, err == nil) - - // Array to read changes from feed to support assertions - var changes = make([]*ChangeEntry, 0, 50) - - err = appendFromFeed(&changes, feed, 4, base.DefaultWaitForSequence) - - // Validate the initial sequences arrive as expected - assert.True(t, err == nil) - assert.Len(t, changes, 4) - assert.Equal(t, &ChangeEntry{ - Seq: SequenceID{Seq: 1, TriggeredBy: 0, LowSeq: 2}, - ID: "doc-1", - collectionID: dbCollection.GetCollectionID(), - Changes: []ChangeRev{{"rev": "1-a"}}}, changes[0]) - - // Test backfill clear - sequence numbers go back to standard handling - WriteDirect(t, collection, []string{"ABC", "NBC", "PBS", "TBS"}, 3) - WriteDirect(t, collection, []string{"ABC", "PBS"}, 4) - - require.NoError(t, db.changeCache.waitForSequenceNotSkipped(ctx, 4, base.DefaultWaitForSequence)) - - err = appendFromFeed(&changes, feed, 2, base.DefaultWaitForSequence) - assert.True(t, err == nil) - assert.Len(t, changes, 6) - // TODO: Test flake: Issue #3056 causing two seq 3 entries and losing seq 4 since we only pull in two more feed items - // {Seq:2::3, ID:doc-3, Changes:[map[rev:1-a]]} {Seq:3, ID:doc-3, Changes:[map[rev:1-a]]} - assert.True(t, verifyChangesSequencesIgnoreOrder(changes, []uint64{1, 2, 5, 6, 3, 4})) - - WriteDirect(t, collection, []string{"ABC"}, 7) - WriteDirect(t, collection, []string{"ABC", "NBC"}, 8) - WriteDirect(t, collection, []string{"ABC", "PBS"}, 9) - require.NoError(t, db.changeCache.waitForSequence(ctx, 9, base.DefaultWaitForSequence)) - newChanges, err := verifySequencesInFeed(feed, []uint64{7, 8, 9}) - require.NoError(t, err) - - assert.True(t, verifyChangesSequencesIgnoreOrder(append(changes, newChanges...), []uint64{1, 2, 5, 6, 3, 4, 7, 8, 9})) -} - // Test race condition causing skipped sequences in changes feed. Channel feeds are processed sequentially // in the main changes.go iteration loop, without a lock on the underlying channel caches. The following // sequence is possible while running a changes feed for channels "A", "B":