Skip to content

Fix v3/roles perf regression for regular users#5157

Merged
johha merged 2 commits into
mainfrom
roles-perf-regression
Jun 9, 2026
Merged

Fix v3/roles perf regression for regular users#5157
johha merged 2 commits into
mainfrom
roles-perf-regression

Conversation

@johha

@johha johha commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a ~2x perf regression on GET /v3/roles (as regular user) introduced by #5094.
Related performance tests results of capi release v1.236.0:
https://github.com/cloudfoundry/cf-performance-tests-pipeline/blob/main/results/rails/postgres/charts/roles-test-results/v1/roles-detailed-chart-with-most-recent-runs.png

Root cause: Membership#authorized_spaces_subquery returns Space.where(id: ...).or(organization_id: ...), which Postgres compiles as a SubPlan re-evaluated once per branch of the Role UNION ALL — 8x Seq Scan onspaces.

Perf measurements

Local repro: 5k orgs / 20k spaces / 50k users / ~880k role rows. Target user has mixed org + space role memberships. Benchmark calls RolesController#readable_roles directly via bin/console and runs EXPLAIN ANALYZE.

State Cost Exec Buffers
1.236.0 (regression) 133401 18.0 ms 2369
#5094 reverted (1.235.0) 131804 4.6 ms 532
This PR 131804 4.6 ms 532

Plan and buffer counts match the reverted state exactly.

Fix

  • membership.rb: rewrite authorized_spaces_subquery / authorized_orgs_subquery
    to push the OR inside a single id IN (UNION). Same return contract; planner
    can now flatten. Benefits every caller of readable_*_query.
  • membership.rb: add authorized_space_ids_subquery / authorized_org_ids_subquery + flat id-only UNIONs without the entity wrapper. Used by callers that filter
    by a raw space_id / organization_id FK column (avoids a redundant subplan over spaces / organizations).
  • permissions.rb: expose readable_space_ids_query / readable_org_ids_query.
  • roles_controller.rb: readable_roles uses the id-only queries.
  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

PR #5094 (commit 0068461) reshaped readable_roles to filter via Space.where(...).or(...) which the planner re-runs as a SubPlan once per role-table append in the Role UNION ALL (8x Seq Scan on spaces).

Related performance test of capi release v1.236.0:
https://github.com/cloudfoundry/cf-performance-tests-pipeline/blob/main/results/rails/postgres/charts/roles-test-results/v1/roles-detailed-chart-with-most-recent-runs.png

Changes:
  - membership.rb: rewrap authorized_spaces/orgs_subquery as `id IN (UNION)`
    so the OR collapses; benefits every caller of readable_*_query
  - membership.rb: add authorized_space/org_ids_subquery returning bare-id
    unions for callers filtering by raw FK columns
  - permissions.rb: expose readable_space/org_ids_query
  - roles_controller.rb: use the id-only queries in readable_roles
@johha johha marked this pull request as ready for review June 8, 2026 17:01

@philippthun philippthun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly cosmetic...

Comment thread lib/cloud_controller/permissions.rb Outdated
Comment thread lib/cloud_controller/permissions.rb Outdated
Comment thread lib/cloud_controller/permissions.rb Outdated
Comment thread lib/cloud_controller/membership.rb Outdated
Comment thread lib/cloud_controller/membership.rb Outdated
Comment thread lib/cloud_controller/membership.rb Outdated
Comment thread lib/cloud_controller/membership.rb Outdated
@johha johha requested a review from philippthun June 9, 2026 10:40
@johha johha force-pushed the roles-perf-regression branch from 17c8f45 to 04393b5 Compare June 9, 2026 10:48
@johha johha merged commit ca32b62 into main Jun 9, 2026
11 checks passed
@johha johha deleted the roles-perf-regression branch June 9, 2026 11:10
ari-wg-gitbot added a commit to cloudfoundry/capi-release that referenced this pull request Jun 9, 2026
Changes in cloud_controller_ng:

- Fix v3/roles perf regression for regular users
    PR: cloudfoundry/cloud_controller_ng#5157
    Author: Johannes Haass <johannes.haass@sap.com>
philippthun added a commit to sap-contributions/cloud_controller_ng that referenced this pull request Jun 10, 2026
PR cloudfoundry#5157 introduced authorized_space_ids_subquery and
authorized_org_ids_subquery. These methods avoid unnecessary subplans on
the spaces / organizations tables on every permission lookup and are now
also used by:

- Permissions#readable_security_group_guids_query
- Permissions#readable_space_quota_guids
- Permissions#is_org_manager?
- Membership#authorized_space_guids_subquery
- Membership#authorized_org_guids_subquery
- Service-credential-binding fetchers
- Service-offering / service-plan list fetchers

The service-offering / service-plan list fetchers now take both
*_ids_query and *_query; the id-query feeds the always-executed
permission filter, the full query is consulted only on guid-filtered
requests.

Also removes the now-dead 'membership' helper from
v3/application_controller.rb (no V3 controller calls it after cloudfoundry#5094).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants