Skip to content

CBG-5286 [3.3.5 backport] fix missing changes with active_only=true and limit=#8231

Merged
torcolvin merged 2 commits intorelease/3.3.5from
CBG-5286
May 6, 2026
Merged

CBG-5286 [3.3.5 backport] fix missing changes with active_only=true and limit=#8231
torcolvin merged 2 commits intorelease/3.3.5from
CBG-5286

Conversation

@gregns1
Copy link
Copy Markdown
Contributor

@gregns1 gregns1 commented May 5, 2026

CBG-5286

Backports #8144

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

  • Link upstream PRs
  • Update Go module dependencies when merged

Integration Tests

Copilot AI review requested due to automatic review settings May 5, 2026 16:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This backport addresses a changes feed edge case where active_only=true combined with a limit could cause GetChanges to omit relevant cached entries (especially when backfill/query results include only channel removals), leading to missing active changes.

Changes:

  • Adjusted singleChannelCacheImpl.GetChanges to always consider appending cache results when ActiveOnly is enabled, and to avoid applying a naive limit at the concatenation step in that mode.
  • Added targeted channel cache tests covering active_only + limit interactions across several scenarios, including “query returns only removals but cache has an active rev”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
db/channel_cache_single.go Ensures cache entries are appended for ActiveOnly requests even when query results exceed the requested limit due to inactive entries.
db/channel_cache_test.go Adds regression tests to validate correct behavior for active_only=true with/without limit across mixed active/removal scenarios.

Comment thread db/channel_cache_test.go
Comment on lines +611 to +620
// doc1: active (seq 1) - will be in backing store, not cache
_, _, _ = collection.Put(ctx, "doc1", Body{"channels": activeChannel})

// doc2: active (seq 2) -> inactive (seq 3) - seq 3 in cache
revID2, _, _ := collection.Put(ctx, "doc2", Body{"channels": activeChannel})
_, _, _ = collection.Put(ctx, "doc2", Body{"channels": "other", "_rev": revID2})

// doc3: active (seq 4) -> inactive (seq 5) - seq 5 in cache
revID3, _, _ := collection.Put(ctx, "doc3", Body{"channels": activeChannel})
_, _, _ = collection.Put(ctx, "doc3", Body{"channels": "other", "_rev": revID3})
Comment thread db/channel_cache_test.go
Comment on lines +647 to +654
_, _, _ = collection.Put(ctx, "doc1", Body{"channels": activeChannel})

// doc2: active (seq 2) -> inactive (seq 3) - cache
revID2, _, _ := collection.Put(ctx, "doc2", Body{"channels": activeChannel})
_, _, _ = collection.Put(ctx, "doc2", Body{"channels": "other", "_rev": revID2})

// doc3: active (seq 4) - cache
_, _, _ = collection.Put(ctx, "doc3", Body{"channels": activeChannel})
Comment thread db/channel_cache_test.go
Comment on lines +682 to +687
revID1, _, _ := collection.Put(ctx, "doc1", Body{"channels": activeChannel})
_, _, _ = collection.Put(ctx, "doc1", Body{"channels": "other", "_rev": revID1})

// doc2: active (seq 3) -> inactive (seq 4)
revID2, _, _ := collection.Put(ctx, "doc2", Body{"channels": activeChannel})
_, _, _ = collection.Put(ctx, "doc2", Body{"channels": "other", "_rev": revID2})
Comment thread db/channel_cache_test.go
Comment on lines +624 to +639
// With limit 1 (before)
changesOptions := ChangesOptions{Since: SequenceID{Seq: 0}, ActiveOnly: true, Limit: 1, ChangesCtx: base.TestCtx(t)}
changes := getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
require.Len(t, changes, 1)
assert.Equal(t, "doc1", changes[0].ID)

// No limit
changesOptions.Limit = 0
changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
require.Len(t, changes, 1)
assert.Equal(t, "doc1", changes[0].ID)

// With limit 1 (after)
changesOptions.Limit = 1
changes = getChanges(t, collection, base.SetOf(activeChannel), changesOptions)
require.Len(t, changes, 1)
@gregns1 gregns1 requested a review from torcolvin May 5, 2026 16:43
@torcolvin torcolvin merged commit bbec338 into release/3.3.5 May 6, 2026
48 checks passed
@torcolvin torcolvin deleted the CBG-5286 branch May 6, 2026 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants