Skip to content

Commit

Permalink
Merge pull request #632 from sapcc/fix-azseparated-sum-quota
Browse files Browse the repository at this point in the history 
SyncQuotaToBackend: fix overall resource quota for AZSeparated resources
  • Loading branch information
@majewsky
majewsky authored Dec 16, 2024
2 parents 514dc11 + bcf81cc commit d9505e5
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 48 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
83 changes: 50 additions & 33 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,50 +142,65 @@ 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
}
targetQuotasInDB[resourceName] = targetQuota
if currentQuota == nil || *currentQuota < 0 || uint64(*currentQuota) != targetQuota {
needsApply = true
}

resInfo := c.Cluster.InfoForResource(srv.Type, resourceName)
if resInfo.Topology != liquid.AZSeparatedResourceTopology {
if !resInfo.HasQuota {
return nil
}
err = sqlext.ForeachRow(c.DB, azQuotaSyncSelectQuery, []any{resourceID}, func(rows *sql.Rows) error {
var (
availabilityZone liquid.AvailabilityZone
currentAZQuota *int64
targetAZQuota uint64
)
err := rows.Scan(&availabilityZone, &currentAZQuota, &targetAZQuota)

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
targetQuota = 0
err = sqlext.ForeachRow(c.DB, azQuotaSyncSelectQuery, []any{resourceID}, func(rows *sql.Rows) error {
var (
availabilityZone liquid.AvailabilityZone
currentAZQuota *int64
targetAZQuota uint64
)
err := rows.Scan(&availabilityZone, &currentAZQuota, &targetAZQuota)
if err != nil {
return err
}
// defense in depth: configured backend_quota for AZ any or unknown are not valid for the azSeparatedQuota topology.
if (availabilityZone == liquid.AvailabilityZoneAny || availabilityZone == liquid.AvailabilityZoneUnknown) && currentAZQuota != nil {
return fmt.Errorf("detected invalid AZ: %s for resource: %s with topology: %s has backend_quota: %v", availabilityZone, resourceName, resInfo.Topology, currentAZQuota)
}
azSeparatedResourceIDs = append(azSeparatedResourceIDs, resourceID)
if targetAZQuotasInDB[resourceName] == nil {
targetAZQuotasInDB[resourceName] = make(map[liquid.AvailabilityZone]liquid.AZResourceQuotaRequest)
}
targetAZQuotasInDB[resourceName][availabilityZone] = liquid.AZResourceQuotaRequest{Quota: targetAZQuota}
targetQuota += targetAZQuota
if currentAZQuota == nil || *currentAZQuota < 0 || uint64(*currentAZQuota) != targetAZQuota {
azSeparatedNeedsApply = true
}
return nil
})
if err != nil {
return err
}
// defense in depth: configured backend_quota for AZ any or unknown are not valid for the azSeparatedQuota topology.
if (availabilityZone == liquid.AvailabilityZoneAny || availabilityZone == liquid.AvailabilityZoneUnknown) && currentAZQuota != nil {
return fmt.Errorf("detected invalid AZ: %s for resource: %s with topology: %s has backend_quota: %v", availabilityZone, resourceName, resInfo.Topology, currentAZQuota)
}
azSeparatedResourceIDs = append(azSeparatedResourceIDs, resourceID)
if targetAZQuotasInDB[resourceName] == nil {
targetAZQuotasInDB[resourceName] = make(map[liquid.AvailabilityZone]liquid.AZResourceQuotaRequest)
}
targetAZQuotasInDB[resourceName][availabilityZone] = liquid.AZResourceQuotaRequest{Quota: targetAZQuota}
if currentAZQuota == nil || *currentAZQuota < 0 || uint64(*currentAZQuota) != targetAZQuota {
azSeparatedNeedsApply = true
} 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)
}
return nil
})
if err != nil {
return err
targetQuota = *targetQuotaPtr
}

targetQuotasInDB[resourceName] = targetQuota
if currentQuota == nil || *currentQuota < 0 || uint64(*currentQuota) != targetQuota {
needsApply = true
}
return nil
})
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 d9505e5

Please sign in to comment.