Skip to content

Commit cb5b6f3

Browse files
jordaneclaude
andcommitted
Scope committee_member.sfid forward mapping key by committee
The v2->v1 create path keyed the forward mapping on the v1 API MemberID (contact SFID) alone, which the same contact reuses across every committee it belongs to. Adding that contact to a second committee silently overwrote the first committee's forward mapping, orphaning it so a later v1-WAL delete couldn't find it. Scope the create-path key by committee SFID and update the delete path to look up the scoped key. Add a one-shot backfill to migrate existing flat keys to the scoped format and tombstone stale entries whose committee can no longer be resolved. Issue: LFXV2-2709 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Jordan Evans <jevans@linuxfoundation.org>
1 parent 099e6b6 commit cb5b6f3

6 files changed

Lines changed: 562 additions & 21 deletions

README.md

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ The following table shows the supported mapping key patterns and their expected
8484
| v1→v2 | `committee.sfid.{v1_sfid}` | `committee.sfid.123e4567-e89b-12d3-a456-426614174003` | `{v2_uuid}` | Committee SFID to UUID |
8585
| v2→v1 | `committee.uid.{v2_uuid}` | `committee.uid.123e4567-e89b-12d3-a456-426614174001` | `{project_sfid}:{committee_sfid}` | Committee UUID to compound SFID |
8686
| **Committee Members** |
87-
| v1→v2 | `committee_member.sfid.{v1_sfid}` | `committee_member.sfid.123e4567-e89b-12d3-a456-426614174004` | `{committee_uuid}:{member_uuid}` | Member SFID to compound UUID |
87+
| v1→v2 | `committee_member.sfid.{v1_sfid}` | `committee_member.sfid.123e4567-e89b-12d3-a456-426614174004` | `{committee_uuid}:{member_uuid}` | Member record SFID to compound UUID |
88+
| v2→v1 | `committee_member.sfid.{committee_sfid}.{member_sfid}` | `committee_member.sfid.a0941000002wBjEAAU.0034100000abcDEAAY` | `{committee_uuid}:{member_uuid}` | Member SFID to compound UUID, scoped by committee since the v1 API MemberID (contact SFID) is reused across committees for the same contact (LFXV2-2709) |
8889
| v2→v1 | `committee_member.uid.{v2_member_uuid}` | `committee_member.uid.123e4567-e89b-12d3-a456-426614174002` | `{project_sfid}:{committee_sfid}:{member_sfid}` | Member UUID to compound SFID |
8990

9091
### User SFID Lookup API
@@ -194,6 +195,35 @@ Locally (with NATS port-forwarded):
194195
lfx-v1-sync-helper --backfill-committee-member-mappings [--dry-run]
195196
```
196197

198+
**Committee-member forward-mapping migration (`--backfill-committee-member-forward-mappings`):**
199+
200+
Migrates committee-member forward mappings written by the v2→v1 create path
201+
(`committee_member.sfid.{v1_sfid}`, keyed only on the contact SFID) to the committee-scoped
202+
format `committee_member.sfid.{committee_sfid}.{member_sfid}` — the fix for LFXV2-2709, where an
203+
unscoped key let MemberID reuse across committees silently overwrite an earlier committee's
204+
mapping. Skips v1-ingest forward keys (record sfid, a UUID) untouched. Because the collision
205+
already collapsed colliding committees' entries into a single flat key before this fix shipped,
206+
only the surviving (last-write) entry per contact can be migrated — this is cleanup, not data
207+
recovery. Writes are optimistic-concurrency guarded against the live sync-helper, idempotent, and
208+
safe to re-run.
209+
210+
Run once after deploying the fix, to migrate already-affected mappings. A dry-run pass is
211+
recommended first:
212+
213+
```sh
214+
kubectl --context lfx-v2-prod -n v1-sync-helper apply -f manifests/backfill-committee-member-forward-mappings-job.yaml
215+
```
216+
217+
Add `--dry-run` to the manifest args, apply, inspect logs
218+
(`inspected`/`migrated`/`skipped_record_key`/`malformed`/`tombstoned`/`unresolved`/`conflicted`
219+
counts), then re-apply without it for the live run.
220+
221+
Locally (with NATS port-forwarded):
222+
223+
```sh
224+
lfx-v1-sync-helper --backfill-committee-member-forward-mappings [--dry-run]
225+
```
226+
197227
## Architecture Diagrams
198228

199229
Regarding the following sequence diagrams:
Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
// Copyright The Linux Foundation and each contributor to LFX.
2+
// SPDX-License-Identifier: MIT
3+
4+
// The lfx-v1-sync-helper service.
5+
package main
6+
7+
import (
8+
"context"
9+
"fmt"
10+
"strings"
11+
12+
"github.com/linuxfoundation/lfx-v1-sync-helper/internal/sfid"
13+
"github.com/nats-io/nats.go/jetstream"
14+
)
15+
16+
// backfillCommitteeMemberForwardMappingsResult summarizes a backfill run.
17+
type backfillCommitteeMemberForwardMappingsResult struct {
18+
inspected int
19+
migrated int
20+
skippedRecordKey int
21+
malformed int
22+
tombstoned int
23+
unresolved int
24+
conflicted int
25+
}
26+
27+
// backfillCommitteeMemberForwardMappings migrates committee-member forward mappings
28+
// written by the v2->v1 create path (syncCommitteeMemberCreateToV1) from the flat key
29+
// "committee_member.sfid.<memberSFID>" to the committee-scoped key
30+
// committeeMemberForwardKey(committeeSFID, memberSFID). See LFXV2-2709.
31+
//
32+
// memberSFID there is the v1 API "MemberID" (the contact SFID), which the same contact
33+
// reuses across every committee it belongs to. Before this fix, the create path's key
34+
// carried no committee scope, so adding the same contact to a second committee silently
35+
// overwrote the first committee's forward mapping under the same key — orphaning it, so
36+
// a later v1-WAL delete for that first committee's member could no longer find it.
37+
//
38+
// This is deliberately kept separate from backfillCommitteeMemberMappings (the
39+
// LFXV2-2673 reverse-mapping repair): different key, different collision, and keeping
40+
// them apart leaves that reviewed backfill untouched.
41+
//
42+
// Enumeration uses ScanSubjectData (sequential GetMsg / next_by_subj), matching the
43+
// reverse-mapping backfill (see its doc comment for why: KV_v1-mappings has ~34M
44+
// sequences and ephemeral-consumer enumeration saturates the NATS server). The single
45+
// trailing wildcard in forwardSubject only matches the flat, single-token key format;
46+
// the newer two-token committee-scoped keys have an extra "." and are never matched, so
47+
// they're never revisited by this backfill.
48+
//
49+
// Writes are guarded by a fresh KV read/CAS at write time, so a concurrent write by the
50+
// live sync-helper is reported as conflicted rather than overwritten.
51+
//
52+
// Known limitation: for a contact whose collision already overwrote an earlier
53+
// committee's forward mapping before this fix shipped, only the surviving (last-write)
54+
// entry is present to migrate — the orphaned committee's original mapping is gone. This
55+
// backfill is cleanup (stop new key collisions, migrate what's left), not recovery of
56+
// data already lost to the bug.
57+
func backfillCommitteeMemberForwardMappings(ctx context.Context, dryRun bool) (backfillCommitteeMemberForwardMappingsResult, error) {
58+
const (
59+
// kvMappingsStream is the JetStream stream backing the v1-mappings KV bucket.
60+
kvMappingsStream = "KV_v1-mappings"
61+
62+
// forwardSubject is the next_by_subj filter for flat committee-member forward
63+
// mappings. A single-token wildcard matches only "committee_member.sfid.<x>"
64+
// (one further dot-free token) — it does not match the newer two-token
65+
// committee-scoped keys "committee_member.sfid.<committeeSFID>.<memberSFID>".
66+
forwardSubject = "$KV.v1-mappings.committee_member.sfid.*"
67+
68+
// forwardPrefix is stripped from the subject to recover the key token.
69+
forwardPrefix = "$KV.v1-mappings.committee_member.sfid."
70+
)
71+
72+
opTimeout := cfg.NATSFetchMaxWait
73+
if opTimeout <= 0 {
74+
opTimeout = defaultNATSFetchMaxWait
75+
}
76+
77+
subjectData, err := ScanSubjectData(ctx, jsContext, kvMappingsStream, forwardSubject, opTimeout)
78+
if err != nil {
79+
return backfillCommitteeMemberForwardMappingsResult{}, fmt.Errorf("failed to scan committee member forward mappings: %w", err)
80+
}
81+
82+
var res backfillCommitteeMemberForwardMappingsResult
83+
for subject, data := range subjectData {
84+
if !strings.HasPrefix(subject, forwardPrefix) {
85+
continue
86+
}
87+
res.inspected++
88+
token := subject[len(forwardPrefix):]
89+
flatKey := "committee_member.sfid." + token
90+
91+
class := classifyForwardMapping(token, data)
92+
switch class.outcome {
93+
case forwardMappingTombstoned:
94+
res.tombstoned++
95+
continue
96+
case forwardMappingRecordKey:
97+
// v1-ingest forward key (handlers_committees.go), keyed on the globally
98+
// unique platform-community__c record sfid; not affected by LFXV2-2709.
99+
res.skippedRecordKey++
100+
continue
101+
case forwardMappingMalformed:
102+
res.malformed++
103+
logger.With("member_sfid", token, "mapping_value", string(data)).
104+
WarnContext(ctx, "skipping malformed committee member forward mapping")
105+
continue
106+
}
107+
108+
// forwardMappingNeedsMigration: token is the contact SFID from a v2->v1
109+
// create-path key; data is "committeeUID:memberUID".
110+
committeeUID, memberUID := class.committeeUID, class.memberUID
111+
log := logger.With("member_sfid", token, "committee_uid", committeeUID, "member_uid", memberUID)
112+
113+
committeeEntry, err := mappingsKV.Get(ctx, "committee.uid."+committeeUID)
114+
if err != nil && err != jetstream.ErrKeyNotFound && err != jetstream.ErrKeyDeleted {
115+
// A transient read failure (timeout, disconnect) is not the same as the
116+
// committee genuinely being gone — treating it as unresolved would
117+
// tombstone the sole surviving mapping over a blip. Abort the run instead.
118+
return res, fmt.Errorf("failed to read committee reverse mapping committee.uid.%s: %w", committeeUID, err)
119+
}
120+
committeeSFID := ""
121+
if err == nil && !isTombstonedMapping(committeeEntry.Value()) {
122+
if _, csfid, ok := splitTwoParts(string(committeeEntry.Value())); ok && csfid != "" {
123+
committeeSFID = csfid
124+
}
125+
}
126+
if committeeSFID == "" {
127+
// The committee this stale mapping points at can no longer be resolved
128+
// (deleted, tombstoned, or malformed reverse mapping) — there's no
129+
// committee to scope the key by, so the stale flat entry is just dead
130+
// weight. Tombstone it rather than leave it around indefinitely.
131+
log.WarnContext(ctx, "cannot resolve committee sfid for stale committee member forward mapping, tombstoning instead of migrating")
132+
if dryRun {
133+
log.InfoContext(ctx, "[dry-run] would tombstone unresolvable committee member forward mapping")
134+
res.unresolved++
135+
continue
136+
}
137+
conflicted, tombErr := tombstoneFlatForwardMappingIfUnchanged(ctx, flatKey, data)
138+
if tombErr != nil {
139+
return res, tombErr
140+
}
141+
if conflicted {
142+
res.conflicted++
143+
continue
144+
}
145+
res.unresolved++
146+
continue
147+
}
148+
149+
scopedKey := committeeMemberForwardKey(committeeSFID, token)
150+
log = log.With("committee_sfid", committeeSFID, "scoped_key", scopedKey)
151+
152+
if dryRun {
153+
log.InfoContext(ctx, "[dry-run] would migrate committee member forward mapping to committee-scoped key")
154+
res.migrated++
155+
continue
156+
}
157+
158+
if conflicted, err := createScopedForwardMapping(ctx, scopedKey, data); err != nil {
159+
return res, err
160+
} else if conflicted {
161+
res.conflicted++
162+
log.WarnContext(ctx, "committee member scoped forward mapping already holds an unexpected value, skipping")
163+
continue
164+
}
165+
166+
conflicted, tombErr := tombstoneFlatForwardMappingIfUnchanged(ctx, flatKey, data)
167+
if tombErr != nil {
168+
return res, tombErr
169+
}
170+
if conflicted {
171+
res.conflicted++
172+
log.WarnContext(ctx, "committee member flat forward mapping changed since scan, skipping tombstone")
173+
continue
174+
}
175+
176+
log.InfoContext(ctx, "migrated committee member forward mapping to committee-scoped key")
177+
res.migrated++
178+
}
179+
180+
return res, nil
181+
}
182+
183+
// createScopedForwardMapping writes value under scopedKey unless it's already present
184+
// with that exact value (idempotent re-run) or holds something else (conflict, don't
185+
// clobber). Returns conflicted=true if the caller should count this as a skip rather
186+
// than treat it as written.
187+
func createScopedForwardMapping(ctx context.Context, scopedKey string, value []byte) (conflicted bool, err error) {
188+
entry, getErr := mappingsKV.Get(ctx, scopedKey)
189+
switch {
190+
case getErr == nil && string(entry.Value()) == string(value):
191+
// Already migrated (e.g. a prior run crashed after this write but before
192+
// tombstoning the flat key, or the live create path wrote it directly).
193+
return false, nil
194+
case getErr == nil:
195+
// Holds something else (tombstoned, or a different committee/member pairing) —
196+
// don't overwrite a value we don't understand.
197+
return true, nil
198+
case getErr != jetstream.ErrKeyNotFound && getErr != jetstream.ErrKeyDeleted:
199+
return false, fmt.Errorf("failed to read committee member scoped forward mapping %s: %w", scopedKey, getErr)
200+
}
201+
202+
if _, err := mappingsKV.Create(ctx, scopedKey, value); err != nil {
203+
if isRevisionMismatchError(err) || err == jetstream.ErrKeyExists {
204+
return true, nil
205+
}
206+
return false, fmt.Errorf("failed to create committee member scoped forward mapping %s: %w", scopedKey, err)
207+
}
208+
return false, nil
209+
}
210+
211+
// tombstoneFlatForwardMappingIfUnchanged revision-guards a tombstone write for a flat
212+
// committee_member.sfid.<token> key: it re-reads the key and only tombstones it if the
213+
// value still matches what the scan observed, treating a value that has since changed
214+
// (or a key that's already gone) as a conflict rather than an error, since the live
215+
// sync-helper or another backfill run may have already handled it.
216+
func tombstoneFlatForwardMappingIfUnchanged(ctx context.Context, flatKey string, expectedValue []byte) (conflicted bool, err error) {
217+
entry, getErr := mappingsKV.Get(ctx, flatKey)
218+
if getErr != nil {
219+
if getErr == jetstream.ErrKeyNotFound || getErr == jetstream.ErrKeyDeleted {
220+
return true, nil
221+
}
222+
return false, fmt.Errorf("failed to re-read committee member forward mapping %s: %w", flatKey, getErr)
223+
}
224+
if string(entry.Value()) != string(expectedValue) {
225+
return true, nil
226+
}
227+
if _, err := mappingsKV.Update(ctx, flatKey, []byte(tombstoneMarker), entry.Revision()); err != nil {
228+
if isRevisionMismatchError(err) {
229+
return true, nil
230+
}
231+
return false, fmt.Errorf("failed to tombstone committee member forward mapping %s: %w", flatKey, err)
232+
}
233+
return false, nil
234+
}
235+
236+
// forwardMappingOutcome is the pre-lookup triage outcome for a scanned flat
237+
// committee-member forward mapping entry, computed without any NATS access so it can be
238+
// unit tested in isolation.
239+
type forwardMappingOutcome string
240+
241+
const (
242+
forwardMappingTombstoned forwardMappingOutcome = "tombstoned"
243+
forwardMappingRecordKey forwardMappingOutcome = "recordKey"
244+
forwardMappingMalformed forwardMappingOutcome = "malformed"
245+
forwardMappingNeedsMigration forwardMappingOutcome = "needsMigration"
246+
)
247+
248+
// forwardMappingClassification is the result of classifyForwardMapping.
249+
type forwardMappingClassification struct {
250+
outcome forwardMappingOutcome
251+
// committeeUID and memberUID are only populated when outcome is
252+
// forwardMappingNeedsMigration.
253+
committeeUID string
254+
memberUID string
255+
}
256+
257+
// classifyForwardMapping triages a scanned flat committee-member forward mapping
258+
// (key token and value) into one of four outcomes, without performing any NATS access:
259+
//
260+
// - forwardMappingTombstoned: explicitly tombstoned value (tombstoneMarker).
261+
// - forwardMappingRecordKey: the key token is a platform-community__c record sfid
262+
// (a UUID) — the v1-ingest forward key, globally unique, unaffected by LFXV2-2709.
263+
// - forwardMappingMalformed: token isn't a UUID or a valid SFID, or the value isn't
264+
// parseable as "committeeUID:memberUID".
265+
// - forwardMappingNeedsMigration: token is a contact SFID from the v2->v1 create
266+
// path; the caller must resolve the committee SFID to migrate it.
267+
func classifyForwardMapping(token string, data []byte) forwardMappingClassification {
268+
if isTombstonedMapping(data) {
269+
return forwardMappingClassification{outcome: forwardMappingTombstoned}
270+
}
271+
if isUUID(token) {
272+
return forwardMappingClassification{outcome: forwardMappingRecordKey}
273+
}
274+
if !sfid.IsValid(token) {
275+
return forwardMappingClassification{outcome: forwardMappingMalformed}
276+
}
277+
committeeUID, memberUID, ok := splitTwoParts(string(data))
278+
if !ok || committeeUID == "" || memberUID == "" {
279+
return forwardMappingClassification{outcome: forwardMappingMalformed}
280+
}
281+
return forwardMappingClassification{
282+
outcome: forwardMappingNeedsMigration,
283+
committeeUID: committeeUID,
284+
memberUID: memberUID,
285+
}
286+
}

0 commit comments

Comments
 (0)