From 14a74b25819dbd3afbd858382200335fdc08ed0c Mon Sep 17 00:00:00 2001 From: Stefan Majewsky Date: Mon, 16 Dec 2024 14:51:58 +0100 Subject: [PATCH 1/2] SyncQuotaToBackend: fix overall resource quota for AZSeparated resources --- internal/collector/sync_quota_to_backend.go | 64 +++++++++++---------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/internal/collector/sync_quota_to_backend.go b/internal/collector/sync_quota_to_backend.go index 470382e5..d7e32589 100644 --- a/internal/collector/sync_quota_to_backend.go +++ b/internal/collector/sync_quota_to_backend.go @@ -149,41 +149,45 @@ func (c *Collector) performQuotaSync(ctx context.Context, srv db.ProjectService, 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 { - 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, ¤tAZQuota, &targetAZQuota) + 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, ¤tAZQuota, &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 - } - return nil - }) - if err != nil { - return err + } + + targetQuotasInDB[resourceName] = targetQuota + if currentQuota == nil || *currentQuota < 0 || uint64(*currentQuota) != targetQuota { + needsApply = true } return nil }) From bcf81ccd271aaff481053b2a9f8d4f36aacf732d Mon Sep 17 00:00:00 2001 From: Stefan Majewsky Date: Mon, 16 Dec 2024 15:20:18 +0100 Subject: [PATCH 2/2] fix handling of project_resources.quota for AZSeparatedResourceTopology --- internal/collector/scrape_test.go | 22 +++++++++------- internal/collector/sync_quota_to_backend.go | 25 ++++++++++++++----- .../datamodel/apply_computed_project_quota.go | 15 ++++++----- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/internal/collector/scrape_test.go b/internal/collector/scrape_test.go index 641e6706..11300394 100644 --- a/internal/collector/scrape_test.go +++ b/internal/collector/scrape_test.go @@ -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) } @@ -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'; `) @@ -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(), diff --git a/internal/collector/sync_quota_to_backend.go b/internal/collector/sync_quota_to_backend.go index d7e32589..0ac311f2 100644 --- a/internal/collector/sync_quota_to_backend.go +++ b/internal/collector/sync_quota_to_backend.go @@ -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 @@ -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, ¤tQuota, &targetQuota) + err := rows.Scan(&resourceID, &resourceName, ¤tQuota, &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 @@ -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 diff --git a/internal/datamodel/apply_computed_project_quota.go b/internal/datamodel/apply_computed_project_quota.go index d8003431..6c683b7d 100644 --- a/internal/datamodel/apply_computed_project_quota.go +++ b/internal/datamodel/apply_computed_project_quota.go @@ -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 := "a + 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