From bcf81ccd271aaff481053b2a9f8d4f36aacf732d Mon Sep 17 00:00:00 2001 From: Stefan Majewsky Date: Mon, 16 Dec 2024 15:20:18 +0100 Subject: [PATCH] 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