Skip to content

Commit

Permalink
fix handling of project_resources.quota for AZSeparatedResourceTopology
Browse files Browse the repository at this point in the history
  • Loading branch information
@majewsky
majewsky committed Dec 16, 2024
1 parent 14a74b2 commit bcf81cc
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 21 deletions.
22 changes: 13 additions & 9 deletions internal/collector/scrape_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -636,12 +636,12 @@ func Test_TopologyScrapes(t *testing.T) {
)

// set some quota acpq values.
// resource level
_, err := s.DB.Exec(`UPDATE project_resources SET quota = $1 WHERE name = $2`, 20, "capacity")
// resource level (ACPQ always writes NULL on this level for AZSeparatedResourceTopology)
_, err := s.DB.Exec(`UPDATE project_resources SET quota = NULL WHERE name = $1`, "capacity")
if err != nil {
t.Fatal(err)
}
_, err = s.DB.Exec(`UPDATE project_resources SET quota = $1 WHERE name = $2`, 13, "things")
_, err = s.DB.Exec(`UPDATE project_resources SET quota = NULL WHERE name = $1`, "things")
if err != nil {
t.Fatal(err)
}
Expand All @@ -668,10 +668,10 @@ func Test_TopologyScrapes(t *testing.T) {
UPDATE project_az_resources SET backend_quota = 13 WHERE id = 7 AND resource_id = 3 AND az = 'az-two';
UPDATE project_az_resources SET backend_quota = 20 WHERE id = 8 AND resource_id = 4 AND az = 'az-one';
UPDATE project_az_resources SET backend_quota = 20 WHERE id = 9 AND resource_id = 4 AND az = 'az-two';
UPDATE project_resources SET backend_quota = 20 WHERE id = 1 AND service_id = 1 AND name = 'capacity';
UPDATE project_resources SET backend_quota = 13 WHERE id = 3 AND service_id = 1 AND name = 'things';
UPDATE project_resources SET backend_quota = 20 WHERE id = 4 AND service_id = 2 AND name = 'capacity';
UPDATE project_resources SET backend_quota = 13 WHERE id = 6 AND service_id = 2 AND name = 'things';
UPDATE project_resources SET backend_quota = NULL WHERE id = 1 AND service_id = 1 AND name = 'capacity';
UPDATE project_resources SET backend_quota = NULL WHERE id = 3 AND service_id = 1 AND name = 'things';
UPDATE project_resources SET backend_quota = NULL WHERE id = 4 AND service_id = 2 AND name = 'capacity';
UPDATE project_resources SET backend_quota = NULL WHERE id = 6 AND service_id = 2 AND name = 'things';
UPDATE project_services SET quota_desynced_at = NULL, quota_sync_duration_secs = 5 WHERE id = 1 AND project_id = 1 AND type = 'unittest';
UPDATE project_services SET quota_desynced_at = NULL, quota_sync_duration_secs = 5 WHERE id = 2 AND project_id = 2 AND type = 'unittest';
`)
Expand All @@ -696,8 +696,12 @@ func Test_TopologyScrapes(t *testing.T) {
UPDATE project_az_resources SET backend_quota = NULL WHERE id = 7 AND resource_id = 3 AND az = 'az-two';
UPDATE project_az_resources SET backend_quota = 50 WHERE id = 8 AND resource_id = 4 AND az = 'az-one';
UPDATE project_az_resources SET backend_quota = 50 WHERE id = 9 AND resource_id = 4 AND az = 'az-two';
UPDATE project_services SET scraped_at = %[1]d, checked_at = %[1]d, next_scrape_at = %[2]d WHERE id = 1 AND project_id = 1 AND type = 'unittest';
UPDATE project_services SET scraped_at = %[3]d, checked_at = %[3]d, next_scrape_at = %[4]d WHERE id = 2 AND project_id = 2 AND type = 'unittest';
UPDATE project_resources SET quota = 0, backend_quota = 40 WHERE id = 1 AND service_id = 1 AND name = 'capacity';
UPDATE project_resources SET quota = 0, backend_quota = 26 WHERE id = 3 AND service_id = 1 AND name = 'things';
UPDATE project_resources SET quota = 0, backend_quota = 40 WHERE id = 4 AND service_id = 2 AND name = 'capacity';
UPDATE project_resources SET quota = 0, backend_quota = 26 WHERE id = 6 AND service_id = 2 AND name = 'things';
UPDATE project_services SET scraped_at = %[1]d, checked_at = %[1]d, next_scrape_at = %[2]d, quota_desynced_at = %[1]d WHERE id = 1 AND project_id = 1 AND type = 'unittest';
UPDATE project_services SET scraped_at = %[3]d, checked_at = %[3]d, next_scrape_at = %[4]d, quota_desynced_at = %[3]d WHERE id = 2 AND project_id = 2 AND type = 'unittest';
`,
checkedAt1.Unix(), checkedAt1.Add(scrapeInterval).Unix(),
checkedAt2.Unix(), checkedAt2.Add(scrapeInterval).Unix(),
Expand Down
25 changes: 19 additions & 6 deletions internal/collector/sync_quota_to_backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,12 @@ func (c *Collector) processQuotaSyncTask(ctx context.Context, srv db.ProjectServ
}

var (
// NOTE: This query does not use `AND quota IS NOT NULL` to filter out NoQuota resources
// because it would also filter out resources with AZSeparatedResourceTopology.
quotaSyncSelectQuery = sqlext.SimplifyWhitespace(`
SELECT id, name, backend_quota, quota
FROM project_resources
WHERE service_id = $1 AND quota IS NOT NULL
WHERE service_id = $1
`)
azQuotaSyncSelectQuery = sqlext.SimplifyWhitespace(`
SELECT az, backend_quota, quota
Expand Down Expand Up @@ -140,17 +142,22 @@ func (c *Collector) performQuotaSync(ctx context.Context, srv db.ProjectService,
var azSeparatedResourceIDs []db.ProjectResourceID
err := sqlext.ForeachRow(c.DB, quotaSyncSelectQuery, []any{srv.ID}, func(rows *sql.Rows) error {
var (
resourceID db.ProjectResourceID
resourceName liquid.ResourceName
currentQuota *int64
targetQuota uint64
resourceID db.ProjectResourceID
resourceName liquid.ResourceName
currentQuota *int64
targetQuotaPtr *uint64
)
err := rows.Scan(&resourceID, &resourceName, &currentQuota, &targetQuota)
err := rows.Scan(&resourceID, &resourceName, &currentQuota, &targetQuotaPtr)
if err != nil {
return err
}

resInfo := c.Cluster.InfoForResource(srv.Type, resourceName)
if !resInfo.HasQuota {
return nil
}

var targetQuota uint64
if resInfo.Topology == liquid.AZSeparatedResourceTopology {
// for AZSeparatedResourceTopology, project_resources.quota is effectively empty (always set to zero)
// and `targetQuota` needs to be computed by summing over project_az_resources.quota
Expand Down Expand Up @@ -183,6 +190,12 @@ func (c *Collector) performQuotaSync(ctx context.Context, srv db.ProjectService,
if err != nil {
return err
}
} else {
// for anything except AZSeparatedResourceTopology, the total target quota is just what we have in project_resources.quota
if targetQuotaPtr == nil {
return fmt.Errorf("found unexpected NULL value in project_resources.quota for id = %d", resourceID)
}
targetQuota = *targetQuotaPtr
}

targetQuotasInDB[resourceName] = targetQuota
Expand Down
15 changes: 9 additions & 6 deletions internal/datamodel/apply_computed_project_quota.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,17 @@ func ApplyComputedProjectQuota(serviceType db.ServiceType, resourceName liquid.R
}

err = sqlext.WithPreparedStatement(tx, acpqUpdateProjectQuotaQuery, func(stmt *sql.Stmt) error {
// Skip resources with AZSeparatedResourceTopology. The quota scrape would receive a resource nil value, while ACPQ calculates qouta.
// This would lead to unnecessary quota syncs with the backend, because backendQuota != quota.
if resInfo.Topology == liquid.AZSeparatedResourceTopology {
return nil
}
for resourceID, quota := range quotasByResourceID {
// Resources with AZSeparatedResourceTopology will report `backendQuota == nil` during scrape.
// If we set anything other than nil here, this would lead to unnecessary quota syncs with the backend,
// because backendQuota != quota.
quotaToWrite := &quota
if resInfo.Topology == liquid.AZSeparatedResourceTopology {
quotaToWrite = nil
}

var serviceID db.ProjectServiceID
err := stmt.QueryRow(quota, resourceID).Scan(&serviceID)
err := stmt.QueryRow(quotaToWrite, resourceID).Scan(&serviceID)
if err == sql.ErrNoRows {
// if quota was not actually changed, do not remember this project service as being stale
continue
Expand Down

0 comments on commit bcf81cc

Please sign in to comment.