|
| 1 | +// Copyright 2012-Present Couchbase, Inc. |
| 2 | +// |
| 3 | +// Use of this software is governed by the Business Source License included |
| 4 | +// in the file licenses/BSL-Couchbase.txt. As of the Change Date specified |
| 5 | +// in that file, in accordance with the Business Source License, use of this |
| 6 | +// software will be governed by the Apache License, Version 2.0, included in |
| 7 | +// the file licenses/APL2.txt. |
| 8 | + |
| 9 | +package changestest |
| 10 | + |
| 11 | +import ( |
| 12 | + "fmt" |
| 13 | + "net/http" |
| 14 | + "testing" |
| 15 | + |
| 16 | + "github.com/couchbase/sync_gateway/db" |
| 17 | + "github.com/couchbase/sync_gateway/rest" |
| 18 | + "github.com/couchbase/sync_gateway/testing/assert" |
| 19 | + "github.com/couchbase/sync_gateway/testing/require" |
| 20 | +) |
| 21 | + |
| 22 | +// These two tests are end-to-end (_changes-level) proofs for CBG-5429 review findings #1 |
| 23 | +// and #2. Both currently FAIL: they assert the correct (no-data-loss) behaviour and |
| 24 | +// demonstrate the regression introduced by the new compound "LowSeq:TriggeredBy:Seq" |
| 25 | +// serialization in db/sequence_id.go:57-59. |
| 26 | +// |
| 27 | +// Root cause (shared by both): |
| 28 | +// - The new serialization keeps LowSeq in the string whenever TriggeredBy > 0, even when |
| 29 | +// LowSeq >= Seq (the normal state of a backfill entry, which re-sends OLD low-sequence |
| 30 | +// docs while a newer sequence is skipped). Old code dropped LowSeq in that case. |
| 31 | +// - That LowSeq round-trips back into the client's `since`. If the feed's lowSequence has |
| 32 | +// changed since the string was emitted (the skip resolved), changes.go:851 does NOT zero |
| 33 | +// it, so options.Since.SafeSequence() == LowSeq. |
| 34 | +// - changesFeed starts the per-channel query at SafeSequence() (changes.go:463), so the |
| 35 | +// query start jumps FORWARD to LowSeq, skipping backfill docs with seq in (Seq, LowSeq]. |
| 36 | +// |
| 37 | +// Each test includes a CONTROL request using the exact string the OLD code would have |
| 38 | +// emitted for the same feed state; the control delivers the backfill doc, isolating the |
| 39 | +// serialization as the cause. |
| 40 | + |
| 41 | +// TestChangesBackfillContinuationSkippedByCompoundLowSeq proves finding #1: a limit-paginated |
| 42 | +// backfill that CONTINUES on a follow-up request skips backfill docs because the compound |
| 43 | +// since starts the channel query at SafeSequence()=LowSeq. |
| 44 | +func TestChangesBackfillContinuationSkippedByCompoundLowSeq(t *testing.T) { |
| 45 | + defer db.SuspendSequenceBatching()() |
| 46 | + pendingMaxWait := uint32(5) // promote pending sequences to the skipped queue quickly |
| 47 | + rt := rest.NewRestTester(t, &rest.RestTesterConfig{ |
| 48 | + SyncFn: `function(doc, oldDoc) {channel(doc.channels);}`, |
| 49 | + DatabaseConfig: &rest.DatabaseConfig{ |
| 50 | + DbConfig: rest.DbConfig{ |
| 51 | + CacheConfig: &rest.CacheConfig{ |
| 52 | + ChannelCacheConfig: &rest.ChannelCacheConfig{MaxWaitPending: &pendingMaxWait}, |
| 53 | + }, |
| 54 | + }, |
| 55 | + }, |
| 56 | + }) |
| 57 | + defer rt.Close() |
| 58 | + testDb := rt.GetDatabase() |
| 59 | + ctx := rt.Context() |
| 60 | + collection, _ := rt.GetSingleTestDatabaseCollection() |
| 61 | + |
| 62 | + rt.CreateUser("sg-user", []string{"ABC"}) // seq 1 |
| 63 | + |
| 64 | + // DEF backfill docs at low seqs 2,3,4. ABC at 5. Gap at 6 (skipped). ABC at 7. |
| 65 | + // The gap makes LowSeq = oldestSkipped-1 = 5, which is > the DEF backfill seqs. |
| 66 | + db.WriteDirect(t, collection, []string{"DEF"}, 2) |
| 67 | + db.WriteDirect(t, collection, []string{"DEF"}, 3) |
| 68 | + db.WriteDirect(t, collection, []string{"DEF"}, 4) |
| 69 | + db.WriteDirect(t, collection, []string{"ABC"}, 5) |
| 70 | + db.WriteDirect(t, collection, []string{"ABC"}, 7) // gap at seq 6 |
| 71 | + rt.WaitForSequenceNotSkipped(7) |
| 72 | + rt.WaitForPendingChanges() |
| 73 | + |
| 74 | + initial := rt.GetChanges("/{{.keyspace}}/_changes", "sg-user") |
| 75 | + require.Equal(t, "5::7", initial.Last_Seq.String()) |
| 76 | + |
| 77 | + // WriteDirect bypasses the sequence allocator, so _sync:seq is still 1. Advance it past the |
| 78 | + // WriteDirect region so the DEF grant (a normal SG write, below) doesn't reuse one of those |
| 79 | + // sequences. The grant then reserves a fresh batch and lands after seq 7 (at maxBatchSize=10 |
| 80 | + // in Rosmar). The exact grant sequence is allocator-batch dependent — and the count of |
| 81 | + // AllocateTestSequence calls does not change it — so we read the resulting since back rather |
| 82 | + // than hard-coding it. (The unused sequences between seq 7 and the grant are a benign artifact.) |
| 83 | + for range 8 { |
| 84 | + _, err := db.AllocateTestSequence(ctx, testDb) |
| 85 | + require.NoError(t, err) |
| 86 | + } |
| 87 | + resp := rt.SendAdminRequest(http.MethodPut, "/{{.db}}/_user/sg-user", |
| 88 | + rest.GetUserPayload(t, "", rest.RestTesterDefaultUserPassword, "", rt.GetSingleDataStore(), []string{"ABC", "DEF"}, nil)) |
| 89 | + rest.RequireStatus(t, resp, http.StatusOK) |
| 90 | + rt.WaitForPendingChanges() |
| 91 | + |
| 92 | + // REQ1: limit=2 splits the DEF backfill mid-stream (sends doc-2, doc-3). Its last_seq is a |
| 93 | + // compound {LowSeq, TriggeredBy, Seq} with LowSeq(5) >= Seq — the corner CBG-5429 changed. |
| 94 | + req1 := rt.PostChanges("/{{.keyspace}}/_changes", fmt.Sprintf(`{"since":"%s","limit":2}`, initial.Last_Seq.String()), "sg-user") |
| 95 | + compound := req1.Last_Seq |
| 96 | + assert.Equal(t, "5:10:3", compound.String()) |
| 97 | + require.Truef(t, compound.LowSeq != 0, "REQ1 last_seq should carry LowSeq while the skip is active; got %q", compound.String()) |
| 98 | + require.Truef(t, compound.TriggeredBy != 0, "REQ1 last_seq should be mid-backfill (TriggeredBy set); got %q", compound.String()) |
| 99 | + require.Truef(t, compound.LowSeq >= compound.Seq, "the affected corner is LowSeq >= Seq; got %q", compound.String()) |
| 100 | + |
| 101 | + // The string OLD code would have emitted for the same value: LowSeq dropped => "TriggeredBy:Seq". |
| 102 | + oldSince := fmt.Sprintf("%d:%d", compound.TriggeredBy, compound.Seq) |
| 103 | + |
| 104 | + // Resolve the skip so the feed's lowSequence changes; the client replays the compound since |
| 105 | + // it just received. changes.go:851 will now NOT zero the LowSeq (it no longer matches lowSequence). |
| 106 | + db.WriteDirect(t, collection, []string{"ABC"}, 6) |
| 107 | + rt.WaitForSequenceNotSkipped(6) |
| 108 | + rt.WaitForPendingChanges() |
| 109 | + |
| 110 | + // REQ2: continue the backfill with the compound since. CONTROL: identical feed state, but with |
| 111 | + // the old-serialization since (LowSeq dropped) — which still delivers the pending backfill doc. |
| 112 | + req2 := rt.PostChanges("/{{.keyspace}}/_changes", fmt.Sprintf(`{"since":"%s","limit":20}`, compound.String()), "sg-user") |
| 113 | + fmt.Println("req", req2.Results) |
| 114 | + control := rt.PostChanges("/{{.keyspace}}/_changes", fmt.Sprintf(`{"since":"%s","limit":20}`, oldSince), "sg-user") |
| 115 | + fmt.Println("old", control.Results) |
| 116 | + |
| 117 | + delivered := changeDocIDSet(req1, req2) |
| 118 | + require.Truef(t, changesHaveDoc(control, "doc-4"), |
| 119 | + "control (old since %q) should deliver the pending DEF backfill doc-4; got %v", oldSince, changeDocIDs(control)) |
| 120 | + require.Truef(t, delivered["doc-4"], |
| 121 | + "FINDING #1: DEF backfill doc-4 was skipped across the paginated compound-since requests (compound since=%q; REQ1=%v REQ2=%v)", |
| 122 | + compound.String(), changeDocIDs(req1), changeDocIDs(req2)) |
| 123 | +} |
| 124 | + |
| 125 | +// TestChangesBackfillGrantSuppressedByCompoundLowSeq proves finding #2: a second channel's |
| 126 | +// FRESH backfill is suppressed because the compound since flips backfillRequired |
| 127 | +// (changes.go:929) to false, so that channel is queried from SafeSequence()=LowSeq. |
| 128 | +func TestChangesBackfillGrantSuppressedByCompoundLowSeq(t *testing.T) { |
| 129 | + pendingMaxWait := uint32(5) |
| 130 | + rt := rest.NewRestTester(t, &rest.RestTesterConfig{ |
| 131 | + SyncFn: `function(doc, oldDoc) {channel(doc.channels);}`, |
| 132 | + DatabaseConfig: &rest.DatabaseConfig{ |
| 133 | + DbConfig: rest.DbConfig{ |
| 134 | + CacheConfig: &rest.CacheConfig{ |
| 135 | + ChannelCacheConfig: &rest.ChannelCacheConfig{MaxWaitPending: &pendingMaxWait}, |
| 136 | + }, |
| 137 | + }, |
| 138 | + }, |
| 139 | + }) |
| 140 | + defer rt.Close() |
| 141 | + testDb := rt.GetDatabase() |
| 142 | + ctx := rt.Context() |
| 143 | + collection, _ := rt.GetSingleTestDatabaseCollection() |
| 144 | + |
| 145 | + rt.CreateUser("sg-user", []string{"ABC"}) // seq 1 |
| 146 | + |
| 147 | + // DEF backfill doc @2, GHI backfill doc @3 (both low). |
| 148 | + db.WriteDirect(t, collection, []string{"DEF"}, 2) |
| 149 | + db.WriteDirect(t, collection, []string{"GHI"}, 3) |
| 150 | + |
| 151 | + // Advance allocator to 3 so the two grants land at seq 4 (DEF) and seq 5 (GHI). |
| 152 | + for range 2 { |
| 153 | + _, err := db.AllocateTestSequence(ctx, testDb) |
| 154 | + require.NoError(t, err) |
| 155 | + } |
| 156 | + resp := rt.SendAdminRequest(http.MethodPut, "/{{.db}}/_user/sg-user", |
| 157 | + rest.GetUserPayload(t, "", rest.RestTesterDefaultUserPassword, "", rt.GetSingleDataStore(), []string{"ABC", "DEF"}, nil)) |
| 158 | + rest.RequireStatus(t, resp, http.StatusOK) // DEF grant -> seqAddedAt 4 |
| 159 | + resp = rt.SendAdminRequest(http.MethodPut, "/{{.db}}/_user/sg-user", |
| 160 | + rest.GetUserPayload(t, "", rest.RestTesterDefaultUserPassword, "", rt.GetSingleDataStore(), []string{"ABC", "DEF", "GHI"}, nil)) |
| 161 | + rest.RequireStatus(t, resp, http.StatusOK) // GHI grant -> seqAddedAt 5 |
| 162 | + |
| 163 | + // ABC @6,7,9 with a gap at 8 => oldestSkipped=8, LowSeq=7. |
| 164 | + // This gives LowSeq(7) >= TriggeredBy(4) and TriggeredBy(4) < GHI grant(5) <= LowSeq(7). |
| 165 | + db.WriteDirect(t, collection, []string{"ABC"}, 6) |
| 166 | + db.WriteDirect(t, collection, []string{"ABC"}, 7) |
| 167 | + db.WriteDirect(t, collection, []string{"ABC"}, 9) |
| 168 | + rt.WaitForSequenceNotSkipped(9) |
| 169 | + |
| 170 | + initChanges := rt.GetChanges("/{{.keyspace}}/_changes?limit=1", "sg-user") |
| 171 | + assert.Equal(t, "7:4:2", initChanges.Last_Seq.String()) |
| 172 | + |
| 173 | + // Resolve the skip so the feed's lowSequence no longer matches the since's LowSeq=7 |
| 174 | + // (otherwise changes.go:851 zeroes it and the flip is masked). The client legitimately |
| 175 | + // replays a "7:4:2" since it received while the skip was still active. |
| 176 | + db.WriteDirect(t, collection, []string{"ABC"}, 8) |
| 177 | + rt.WaitForSequenceNotSkipped(8) |
| 178 | + rt.WaitForPendingChanges() |
| 179 | + |
| 180 | + // "7:4:2" = {LowSeq:7, TriggeredBy:4 (mid DEF backfill), Seq:2} — the compound since the new |
| 181 | + // code emits. "4:2" = what the old code emitted for the same state (LowSeq dropped). |
| 182 | + bug := rt.PostChanges("/{{.keyspace}}/_changes", `{"since":"7:4:2"}`, "sg-user") |
| 183 | + control := rt.PostChanges("/{{.keyspace}}/_changes", `{"since":"4:2"}`, "sg-user") |
| 184 | + |
| 185 | + fmt.Println("bug", bug.Results) |
| 186 | + fmt.Println("control", control.Results) |
| 187 | + |
| 188 | + require.Truef(t, changesHaveDoc(control, "doc-3"), |
| 189 | + "control (old since \"4:2\") should deliver the GHI backfill doc-3; got %v", changeDocIDs(control)) |
| 190 | + require.Truef(t, changesHaveDoc(bug, "doc-3"), |
| 191 | + "FINDING #2: GHI backfill doc-3 was skipped with the compound since \"7:4:2\" (got %v) — its fresh backfill was suppressed by the backfillRequired flip", |
| 192 | + changeDocIDs(bug)) |
| 193 | +} |
| 194 | + |
| 195 | +func changeDocIDs(cr rest.ChangesResults) []string { |
| 196 | + out := make([]string, 0, len(cr.Results)) |
| 197 | + for _, r := range cr.Results { |
| 198 | + out = append(out, r.ID) |
| 199 | + } |
| 200 | + return out |
| 201 | +} |
| 202 | + |
| 203 | +func changeDocIDSet(crs ...rest.ChangesResults) map[string]bool { |
| 204 | + set := map[string]bool{} |
| 205 | + for _, cr := range crs { |
| 206 | + for _, r := range cr.Results { |
| 207 | + set[r.ID] = true |
| 208 | + } |
| 209 | + } |
| 210 | + return set |
| 211 | +} |
| 212 | + |
| 213 | +func changesHaveDoc(cr rest.ChangesResults, id string) bool { |
| 214 | + for _, r := range cr.Results { |
| 215 | + if r.ID == id { |
| 216 | + return true |
| 217 | + } |
| 218 | + } |
| 219 | + return false |
| 220 | +} |
0 commit comments