Skip to content

[usage] reduce possibility for races in ResetUsage #15006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions components/gitpod-db/go/cost_center.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (c *CostCenterManager) GetOrCreateCostCenter(ctx context.Context, attributi
// This can happen in the following scenario:
// * User accesses gitpod just after their CostCenter expired, but just before our periodic CostCenter reset kicks in.
if result.BillingStrategy == CostCenter_Other && result.IsExpired() {
cc, err := c.ResetUsage(ctx, result)
cc, err := c.ResetUsage(ctx, result.ID)
if err != nil {
logger.WithError(err).Error("Failed to reset expired usage.")
return CostCenter{}, fmt.Errorf("failed to reset usage for expired cost center ID: %s: %w", result.ID, err)
Expand Down Expand Up @@ -262,31 +262,37 @@ func (c *CostCenterManager) ListLatestCostCentersWithBillingTimeBefore(ctx conte
return results, nil
}

func (c *CostCenterManager) ResetUsage(ctx context.Context, cc CostCenter) (CostCenter, error) {
func (c *CostCenterManager) ResetUsage(ctx context.Context, id AttributionID) (CostCenter, error) {
logger := log.WithField("attribution_id", id)
now := time.Now().UTC()
cc, err := getCostCenter(ctx, c.conn, id)
if err != nil {
return cc, err
}
logger = logger.WithField("cost_center", cc)
if cc.BillingStrategy != CostCenter_Other {
return CostCenter{}, fmt.Errorf("cannot reset usage for Billing Strategy %s for Cost Center ID: %s", cc.BillingStrategy, cc.ID)
}
if !cc.IsExpired() {
logger.Info("Skipping ResetUsage because next billing cycle is in the future.")
return cc, nil
}

now := time.Now().UTC()

logger.Info("Running `ResetUsage`.")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.Info("Running `ResetUsage`.")
logger.WithField("attribution_id", id).WithField("cost_center", cc).Infof("Resetting usage for Attribution ID %s", id)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Added the cost center. the attributiond id was already included and I like to keep the message static, as it's a bit easier to filter on that way.

// Default to 1 month from now, if there's no nextBillingTime set on the record.
billingCycleStart := now
nextBillingTime := now.AddDate(0, 1, 0)
if cc.NextBillingTime.IsSet() {
billingCycleStart = cc.NextBillingTime.Time()
nextBillingTime = cc.NextBillingTime.Time().AddDate(0, 1, 0)
}

// Create a synthetic Invoice Usage record, to reset usage
err := c.BalanceOutUsage(ctx, cc.ID)
if err != nil {
return CostCenter{}, fmt.Errorf("failed to compute invocie usage record for AttributonID: %s: %w", cc.ID, err)
}

// All fields on the new cost center remain the same, except for BillingCycleStart, NextBillingTime, and CreationTime
newCostCenter := CostCenter{
ID: cc.ID,
SpendingLimit: cc.SpendingLimit,
BillingStrategy: cc.BillingStrategy,
BillingCycleStart: NewVarCharTime(now),
BillingCycleStart: NewVarCharTime(billingCycleStart),
NextBillingTime: NewVarCharTime(nextBillingTime),
CreationTime: NewVarCharTime(now),
}
Expand All @@ -295,5 +301,11 @@ func (c *CostCenterManager) ResetUsage(ctx context.Context, cc CostCenter) (Cost
return CostCenter{}, fmt.Errorf("failed to store new cost center for AttribtuonID: %s: %w", cc.ID, err)
}

// Create a synthetic Invoice Usage record, to reset usage
err = c.BalanceOutUsage(ctx, cc.ID)
if err != nil {
return CostCenter{}, fmt.Errorf("failed to compute invocie usage record for AttributonID: %s: %w", cc.ID, err)
}

return newCostCenter, nil
}
31 changes: 15 additions & 16 deletions components/gitpod-db/go/cost_center_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,12 +408,14 @@ func TestCostCenterManager_ResetUsage(t *testing.T) {
ForTeams: 0,
ForUsers: 500,
})
_, err := mnr.ResetUsage(context.Background(), db.CostCenter{
cc := dbtest.CreateCostCenters(t, conn, db.CostCenter{
ID: db.NewUserAttributionID(uuid.New().String()),
CreationTime: db.NewVarCharTime(time.Now()),
SpendingLimit: 500,
BillingStrategy: db.CostCenter_Stripe,
})
})[0]

_, err := mnr.ResetUsage(context.Background(), cc.ID)
require.Error(t, err)
})

Expand All @@ -423,23 +425,20 @@ func TestCostCenterManager_ResetUsage(t *testing.T) {
ForTeams: 0,
ForUsers: 500,
})
oldCC := db.CostCenter{
ID: db.NewTeamAttributionID(uuid.New().String()),
CreationTime: db.NewVarCharTime(time.Now()),
SpendingLimit: 0,
BillingStrategy: db.CostCenter_Other,
NextBillingTime: db.NewVarCharTime(ts),
}
newCC, err := mnr.ResetUsage(context.Background(), oldCC)
oldCC := dbtest.CreateCostCenters(t, conn, db.CostCenter{
ID: db.NewTeamAttributionID(uuid.New().String()),
CreationTime: db.NewVarCharTime(time.Now()),
SpendingLimit: 10,
BillingStrategy: db.CostCenter_Other,
NextBillingTime: db.NewVarCharTime(ts),
BillingCycleStart: db.NewVarCharTime(ts.AddDate(0, -1, 0)),
})[0]
newCC, err := mnr.ResetUsage(context.Background(), oldCC.ID)
require.NoError(t, err)
t.Cleanup(func() {
conn.Model(&db.CostCenter{}).Delete(newCC)
})

require.Equal(t, oldCC.ID, newCC.ID)
require.EqualValues(t, 0, newCC.SpendingLimit)
require.EqualValues(t, 10, newCC.SpendingLimit)
require.Equal(t, db.CostCenter_Other, newCC.BillingStrategy)
require.Equal(t, ts.AddDate(0, 1, 0), newCC.NextBillingTime.Time())
require.Equal(t, db.NewVarCharTime(ts.AddDate(0, 1, 0)).Time(), newCC.NextBillingTime.Time())

})

Expand Down
2 changes: 1 addition & 1 deletion components/usage/pkg/apiv1/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func (s *UsageService) ResetUsage(ctx context.Context, req *v1.ResetUsageRequest

var errors []error
for _, cc := range costCentersToUpdate {
_, err = s.costCenterManager.ResetUsage(ctx, cc)
_, err = s.costCenterManager.ResetUsage(ctx, cc.ID)
if err != nil {
errors = append(errors, err)
}
Expand Down