From f45e82b75549b62282960b9865019f8e859df67b Mon Sep 17 00:00:00 2001 From: Bjorn Date: Thu, 21 May 2026 11:36:01 -0700 Subject: [PATCH] sync: pre-build entitlement-id set in processGrantsWithExternalPrincipals Per-grant inner-loop scan over entitlements was O(N) per grant; total O(M*N) where M = grants, N = entitlements per resource. C1 uplift hits this path on every external-principal grant during bucketing, which at the tenant scale represented in c1-uplift-perf-r1.md (millions of grants per Klaviyo / Lilly-class sync) accounts for material wall-clock during post-sync uplift. Pre-build the entitlement-id set once per resource. Inner check becomes O(1). Total O(M + N) per resource. No interface changes, no behavior change for any grant; same iteration order, same side effects. Existing TestExternalResource* coverage continues to pass. Driven by c1 uplift perf campaign tracker candidate (see investigations/c1-uplift-perf-tracker.md in stargate). Sibling change to PR #849 (compareProto -> proto.Equal) in the same campaign. --- pkg/sync/syncer.go | 54 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/pkg/sync/syncer.go b/pkg/sync/syncer.go index 9f3b9c941..cd9ff7c6f 100644 --- a/pkg/sync/syncer.go +++ b/pkg/sync/syncer.go @@ -2150,6 +2150,31 @@ func (s *syncer) listExternalResourcesForResourceType(ctx context.Context, resou return resources, nil } +// listAllEntitlementIDs paginates every entitlement in the local store and +// returns just the ID set. processGrantsWithExternalPrincipals needs only +// existence, not the full entitlement blob, so this avoids the per-slug +// GetEntitlement N+1 inside its nested match-by-id / match-by-key loops. +func (s *syncer) listAllEntitlementIDs(ctx context.Context) (mapset.Set[string], error) { + ids := mapset.NewSet[string]() + pageToken := "" + for { + resp, err := s.store.ListEntitlements(ctx, v2.EntitlementsServiceListEntitlementsRequest_builder{ + PageToken: pageToken, + }.Build()) + if err != nil { + return nil, err + } + for _, ent := range resp.GetList() { + ids.Add(ent.GetId()) + } + pageToken = resp.GetNextPageToken() + if pageToken == "" { + break + } + } + return ids, nil +} + func (s *syncer) listExternalEntitlementsForResource(ctx context.Context, resource *v2.Resource) ([]*v2.Entitlement, error) { ents := make([]*v2.Entitlement, 0) entitlementToken := "" @@ -2238,6 +2263,15 @@ func (s *syncer) processGrantsWithExternalPrincipals(ctx context.Context, princi l := ctxzap.Extract(ctx) + // One ListEntitlements paginate replaces the per-slug GetEntitlement + // existence checks below. Entitlement counts are 10^2-10^4 on real + // tenants, so the in-memory set fits comfortably and the grant-iteration + // loop's inner GetEntitlement fanout collapses to set membership. + knownEntitlementIDs, err := s.listAllEntitlementIDs(ctx) + if err != nil { + return err + } + groupPrincipals := make([]*v2.Resource, 0) userPrincipals := make([]*v2.Resource, 0) principalMap := make(map[string]*v2.Resource) @@ -2347,13 +2381,9 @@ func (s *syncer) processGrantsWithExternalPrincipals(ctx context.Context, princi principalEntitlementSlugs := expandableEntitlementsResourceMap[groupPrincipalBID] for _, slug := range principalEntitlementSlugs { newExpandableEntId := entitlement.NewEntitlementID(principal, slug) - _, err := s.store.GetEntitlement(ctx, reader_v2.EntitlementsReaderServiceGetEntitlementRequest_builder{EntitlementId: newExpandableEntId}.Build()) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - l.Error("found no entitlement with entitlement id generated from external source sync", zap.Any("entitlementId", newExpandableEntId)) - continue - } - return err + if !knownEntitlementIDs.ContainsOne(newExpandableEntId) { + l.Error("found no entitlement with entitlement id generated from external source sync", zap.Any("entitlementId", newExpandableEntId)) + continue } newExpandableEntitlementIDs = append(newExpandableEntitlementIDs, newExpandableEntId) } @@ -2426,13 +2456,9 @@ func (s *syncer) processGrantsWithExternalPrincipals(ctx context.Context, princi principalEntitlementSlugs := expandableEntitlementsResourceMap[groupPrincipalBID] for _, slug := range principalEntitlementSlugs { newExpandableEntId := entitlement.NewEntitlementID(groupPrincipal, slug) - _, err := s.store.GetEntitlement(ctx, reader_v2.EntitlementsReaderServiceGetEntitlementRequest_builder{EntitlementId: newExpandableEntId}.Build()) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - l.Error("found no entitlement with entitlement id generated from external source sync", zap.Any("entitlementId", newExpandableEntId)) - continue - } - return err + if !knownEntitlementIDs.ContainsOne(newExpandableEntId) { + l.Error("found no entitlement with entitlement id generated from external source sync", zap.Any("entitlementId", newExpandableEntId)) + continue } newExpandableEntitlementIDs = append(newExpandableEntitlementIDs, newExpandableEntId) }