Skip to content
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.

Sign up for GitHub

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

SyncQuotaToBackend: fix overall resource quota for AZSeparated resources #632

Merged
merged 2 commits into from
Dec 16, 2024
+72 −48
Merged

SyncQuotaToBackend: fix overall resource quota for AZSeparated resources #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14a74b2
SyncQuotaToBackend: fix overall resource quota for AZSeparated resources
majewsky Dec 16, 2024
bcf81cc
fix handling of project_resources.quota for AZSeparatedResourceTopology
majewsky Dec 16, 2024
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
22 changes: 13 additions & 9 deletions internal/collector/scrape_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading