Skip to content

Rewrite permission queries in domain_fetcher and route_fetcher to use UNION ALL - #5534

Open
WeiQuan0605 wants to merge 1 commit into
mainfrom
Permission-query-perf
Open

WeiQuan0605 wants to merge 1 commit into
mainfrom
Permission-query-perf

Conversation

@WeiQuan0605

Copy link
Copy Markdown
Contributor
  • A short explanation of the proposed change:
    Rewrite the permission queries in DomainFetcher#fetch_all_for_orgs and RouteFetcher#fetch to use UNION ALL subqueries instead of OR predicates. Also adds space_ids_with_readable_routes_query to Permissions and passes readable_space_ids_dataset to RouteFetcher.fetch to avoid a guid→id round-trip.

  • An explanation of the use cases your change solves
    GET /v3/domains and GET /v3/routes for non-admin users were generating correlated SubPlans in Postgres, the permission subquery was re-executed once per row in the outer result set, causing O(n²) behaviour at scale.
    The UNION ALL shape builds hash tables once and eliminates the SubPlans entirely, reducing both queries to O(n).

    GET /v3/domains (domain_fetcher)
    Benchmarked with 20,002 orgs, 101 shared domains, 390 private domains, using EXPLAIN (ANALYZE, BUFFERS)

    Shape Avg execution time Query plan
    OR predicate 41 ms Seq Scan + correlated SubPlan
    UNION ALL 4.6 ms Hash Join via Append
    Improvement ~9x faster

    GET /v3/routes (route_fetcher)
    Benchmarked with 5,001 spaces,10,002 routes,20,000 orgs, using EXPLAIN (ANALYZE, BUFFERS

    Shape Avg execution time Query plan
    OR predicate 5123 ms Seq Scan + correlated SubPlan
    UNION ALL 94 ms Hash Join via Append
    Improvement ~54x faster
  • Links to any other associated PRs

  • 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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant