diff --git a/pkg/appview/db/leases.go b/pkg/appview/db/leases.go index c75d092..e3bf7d1 100644 --- a/pkg/appview/db/leases.go +++ b/pkg/appview/db/leases.go @@ -10,10 +10,9 @@ import ( // Lease names used by the AppView. Each names exactly one background worker // that must not run on more than one instance at a time. const ( - LeaseJetstream = "jetstream" - LeaseBackfill = "backfill" - LeaseCleanup = "cleanup" - LeaseBillingTiers = "billing-tiers" + LeaseJetstream = "jetstream" + LeaseBackfill = "backfill" + LeaseCleanup = "cleanup" ) // LeaseLabeler returns the lease name for a labeler subscriber. Each labeler has diff --git a/pkg/appview/server.go b/pkg/appview/server.go index b5bb4f7..eee70d2 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -310,13 +310,12 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, return nil, fmt.Errorf("billing is enabled but STRIPE_WEBHOOK_SECRET is not set; refusing to start with a forgeable webhook endpoint") } slog.Info("Billing enabled", "appview_did", appviewDID, "managed_holds", len(cfg.Server.ManagedHolds)) - // Leased: RefreshHoldTiers writes tier state derived from Stripe, and - // several instances refreshing the same holds concurrently would race - // on those writes for no benefit. - s.Leases.Go(workerCtx, db.LeaseBillingTiers, func(context.Context) error { - s.BillingManager.RefreshHoldTiers() - return nil // one-shot; the lease is released when it returns - }) + // Deliberately not leased. holdTierCache is per-process memory fed by + // read-only ListTiers calls, so there is nothing to serialise, and + // electing one refresher would leave every other instance reporting + // "no hold data" from aggregateHoldFeatures. Same reasoning as the + // hold health worker above. + go s.BillingManager.RefreshHoldTiers(workerCtx) } // Create webhook dispatcher diff --git a/pkg/billing/billing.go b/pkg/billing/billing.go index 7737a71..8bc449a 100644 --- a/pkg/billing/billing.go +++ b/pkg/billing/billing.go @@ -952,11 +952,20 @@ func (m *Manager) cacheCustomer(userDID string, cust *stripe.Customer) { const holdTierCacheTTL = 30 * time.Minute -// RefreshHoldTiers queries all managed holds for their tier definitions and caches the results. -// It runs once immediately (with retries for holds that aren't ready yet) and then -// periodically in the background. -// Safe to call from a goroutine. -func (m *Manager) RefreshHoldTiers() { +// RefreshHoldTiers queries all managed holds for their tier definitions and +// caches the results. It runs once immediately (with retries for holds that are +// not ready yet) and then periodically until ctx is cancelled. +// +// This runs on every instance rather than under a lease. holdTierCache is +// per-process memory and refreshHoldTiersOnce issues read-only ListTiers calls, +// so there is no shared state to serialise and nothing to race on. Electing one +// refresher would leave every other instance with an empty cache, which +// aggregateHoldFeatures reports as "no hold data" — the same reasoning that +// keeps the hold health worker unleased. +// +// It only returns when ctx is cancelled, so start it with `go` and hand it a +// context that shutdown closes. +func (m *Manager) RefreshHoldTiers(ctx context.Context) { if !m.Enabled() || len(m.managedHolds) == 0 { return } @@ -967,7 +976,7 @@ func (m *Manager) RefreshHoldTiers() { const initialDelay = 3 * time.Second for attempt := range maxRetries { - m.refreshHoldTiersOnce() + m.refreshHoldTiersOnce(ctx) // Check if all managed holds are cached m.holdTierCacheMu.RLock() @@ -982,20 +991,31 @@ func (m *Manager) RefreshHoldTiers() { delay := initialDelay * time.Duration(1<