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.

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

slow /v3/roles call. #278

Closed
pusherofbrooms opened this issue Oct 20, 2022 · 9 comments
Closed

slow /v3/roles call. #278

pusherofbrooms opened this issue Oct 20, 2022 · 9 comments

Comments

@pusherofbrooms
Copy link

pusherofbrooms commented Oct 20, 2022

Issue

/v3/roles calls which look like:
/v3/roles?types=space_developer&space_guids=90775432-527f-4771-a1cc-17a4751de041&user_guids=26c20682-0ec5-4e13-817d-f64d6f580563 seem to be expensive if multiple of them stack up.

Context

Our environment has 231 organizations, several thousand users, and about 20TB of applications running.
cf-deployment version is v21.9.0
CAPI version is v1.136.0

The query in the database looks like this:

SELECT * FROM (SELECT 'organization_user' AS `type`, `role_guid` AS `guid`, `user_id`, `organization_id`, -1 AS `space_id`, `created_at`, `updated_at` FROM `organizations_users` UNION ALL SELECT 'organization_manager' AS `type`, `role_guid` AS `guid`, `user_id`, `organization_id`, -1 AS `space_id`, `created_at`, `updated_at` FROM `organizations_managers` UNION ALL SELECT 'organization_billing_manager' AS `type`, `role_guid` AS `guid`, `user_id`, `organization_id`, -1 AS `space_id`, `created_at`, `updated_at` FROM `organizations_billing_managers` UNION ALL SELECT 'organization_auditor' AS `type`, `role_guid` AS `guid`, `user_id`, `organization_id`, -1 AS `space_id`, `created_at`, `updated_at` FROM `organizations_auditors` UNION ALL SELECT 'space_developer' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_developers` UNION ALL SELECT 'space_auditor' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_auditors` UNION ALL SELECT 'space_supporter' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_supporters` UNION ALL SELECT 'space_manager' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_managers`) AS `t1` WHERE (((`space_id` IN (SELECT `id` FROM (SELECT * FROM (SELECT * FROM (SELECT * FROM (SELECT `spaces`.`id` FROM `spaces` INNER JOIN `spaces_developers` ON ((`spaces_developers`.`space_id` = `spaces`.`id`) AND (`spaces_developers`.`user_id` = 1924)) UNION SELECT `spaces`.`id` FROM `spaces` INNER JOIN `spaces_managers` ON ((`spaces_managers`.`space_id` = `spaces`.`id`) AND (`spaces_managers`.`user_id` = 1924))) AS `t1` UNION SELECT `spaces`.`id` FROM `spaces` INNER JOIN `spaces_auditors` ON ((`spaces_auditors`.`space_id` = `spaces`.`id`) AND (`spaces_auditors`.`user_id` = 1924))) AS `t1` UNION SELECT `spaces`.`id` FROM `spaces` INNER JOIN `spaces_supporters` ON ((`spaces_supporters`.`space_id` = `spaces`.`id`) AND (`spaces_supporters`.`user_id` = 1924))) AS `t1` UNION SELECT `spaces`.`id` FROM `spaces` INNER JOIN `organizations_managers` ON ((`organizations_managers`.`organization_id` = `spaces`.`organization_id`) AND (`organizations_managers`.`user_id` = 1924))) AS `t1`)) OR (`organization_id` IN (SELECT `id` FROM (SELECT * FROM (SELECT * FROM (SELECT `organizations`.`id` FROM `organizations` INNER JOIN `organizations_managers` ON ((`organizations_managers`.`organization_id` = `organizations`.`id`) AND (`organizations_managers`.`user_id` = 1924)) UNION SELECT `organizations`.`id` FROM `organizations` INNER JOIN `organizations_users` ON ((`organizations_users`.`organization_id` = `organizations`.`id`) AND (`organizations_users`.`user_id` = 1924))) AS `t1` UNION SELECT `organizations`.`id` FROM `organizations` INNER JOIN `organizations_billing_managers` ON ((`organizations_billing_managers`.`organization_id` = `organizations`.`id`) AND (`organizations_billing_managers`.`user_id` = 1924))) AS `t1` UNION SELECT `organizations`.`id` FROM `organizations` INNER JOIN `organizations_auditors` ON ((`organizations_auditors`.`organization_id` = `organizations`.`id`) AND (`organizations_auditors`.`user_id` = 1924))) AS `t1`))) AND (`type` IN ('space_developer')) AND (`user_id` IN (SELECT `id` FROM `users` WHERE (`guid` IN ('26c20682-0ec5-4e13-817d-f64d6f580563')))) AND (`space_id` IN (SELECT `id` FROM `spaces` WHERE (`guid` IN ('90775432-527f-4771-a1cc-17a4751de041'))))) ORDER BY `t1`.`created_at` ASC, `t1`.`guid` ASC LIMIT 50 OFFSET 0

At least I think this is the related query.

Steps to Reproduce

I'm not sure how to reproduce it yet except to say this only happens in our largest environment with many concurrent queries.

Expected result

swift return of the query, like less than a second.

Current result

The call can take up to 30 seconds to return if many are stacked up.

Possible Fix

I can't give a fix, but I suspect the related query has an unindexed field.

@sethboyles
Copy link
Member

ran this though a formatter:

SELECT *
FROM   (SELECT 'organization_user' AS `type`,
               `role_guid`         AS `guid`,
               `user_id`,
               `organization_id`,
               -1                  AS `space_id`,
               `created_at`,
               `updated_at`
        FROM   `organizations_users`
        UNION ALL
        SELECT 'organization_manager' AS `type`,
               `role_guid`            AS `guid`,
               `user_id`,
               `organization_id`,
               -1                     AS `space_id`,
               `created_at`,
               `updated_at`
        FROM   `organizations_managers`
        UNION ALL
        SELECT 'organization_billing_manager' AS `type`,
               `role_guid`                    AS `guid`,
               `user_id`,
               `organization_id`,
               -1                             AS `space_id`,
               `created_at`,
               `updated_at`
        FROM   `organizations_billing_managers`
        UNION ALL
        SELECT 'organization_auditor' AS `type`,
               `role_guid`            AS `guid`,
               `user_id`,
               `organization_id`,
               -1                     AS `space_id`,
               `created_at`,
               `updated_at`
        FROM   `organizations_auditors`
        UNION ALL
        SELECT 'space_developer' AS `type`,
               `role_guid`       AS `guid`,
               `user_id`,
               -1                AS `organization_id`,
               `space_id`,
               `created_at`,
               `updated_at`
        FROM   `spaces_developers`
        UNION ALL
        SELECT 'space_auditor' AS `type`,
               `role_guid`     AS `guid`,
               `user_id`,
               -1              AS `organization_id`,
               `space_id`,
               `created_at`,
               `updated_at`
        FROM   `spaces_auditors`
        UNION ALL
        SELECT 'space_supporter' AS `type`,
               `role_guid`       AS `guid`,
               `user_id`,
               -1                AS `organization_id`,
               `space_id`,
               `created_at`,
               `updated_at`
        FROM   `spaces_supporters`
        UNION ALL
        SELECT 'space_manager' AS `type`,
               `role_guid`     AS `guid`,
               `user_id`,
               -1              AS `organization_id`,
               `space_id`,
               `created_at`,
               `updated_at`
        FROM   `spaces_managers`) AS `t1`
WHERE  ( ( ( `space_id` IN (SELECT `id`
                            FROM   (SELECT *
                                    FROM   (SELECT *
                                            FROM   (SELECT *
                                                    FROM   (SELECT `spaces`.`id`
                                                            FROM   `spaces`
                                    INNER JOIN `spaces_developers`
                                            ON ( (
                                    `spaces_developers`.`space_id` =
                                    `spaces`.`id` )
                                                 AND (
                                    `spaces_developers`.`user_id`
                                    = 1924
                                                     )
                                               )
                                            UNION
                                            SELECT `spaces`.`id`
                                            FROM   `spaces`
                                    INNER JOIN `spaces_managers`
                                            ON ( (
                                    `spaces_managers`.`space_id` =
                                    `spaces`.`id` )
                                                 AND (
                                    `spaces_managers`.`user_id` =
                                    1924 )
                                               )
                                           ) AS
                                           `t1`
                                    UNION
                                    SELECT `spaces`.`id`
                                    FROM   `spaces`
                                           INNER JOIN `spaces_auditors`
                                                   ON ( (
                                           `spaces_auditors`.`space_id`
                                           =
                                           `spaces`.`id` )
                                                        AND (
                                           `spaces_auditors`.`user_id`
                                           =
                                           1924 )
                                                      )
                                                   ) AS
                                                   `t1`
                                            UNION
                                            SELECT `spaces`.`id`
                                            FROM   `spaces`
                                    INNER JOIN `spaces_supporters`
                                            ON ( (
                                    `spaces_supporters`.`space_id` =
                                    `spaces`.`id` )
                                                 AND (
                                    `spaces_supporters`.`user_id` = 1924
                                                     ) )) AS
                                           `t1`
                                    UNION
                                    SELECT `spaces`.`id`
                                    FROM   `spaces`
                                           INNER JOIN `organizations_managers`
                                                   ON ( (
                           `organizations_managers`.`organization_id` =
                           `spaces`.`organization_id` )
                                        AND (
                           `organizations_managers`.`user_id` = 1924 )
                                      )
                                   ) AS
                                   `t1`) )
            OR ( `organization_id` IN (SELECT `id`
                                       FROM   (SELECT *
                                               FROM   (
                                      SELECT *
                                      FROM   (SELECT
                                      `organizations`.`id`
                                              FROM   `organizations`
                                      INNER JOIN `organizations_managers`
                                              ON ( (
                 `organizations_managers`.`organization_id`
                 =
                              `organizations`.`id` )
                              AND (
                 `organizations_managers`.`user_id` =
                 1924 ) )
                         UNION
                         SELECT `organizations`.`id`
                         FROM   `organizations`
                 INNER JOIN `organizations_users`
                         ON ( (
                 `organizations_users`.`organization_id` =
                 `organizations`.`id` )
                              AND (
                 `organizations_users`.`user_id` =
                 1924
                                  ) ))
                        AS
                        `t1`
                 UNION
                 SELECT `organizations`.`id`
                 FROM   `organizations`
                 INNER JOIN `organizations_billing_managers`
                         ON ( (
                     `organizations_billing_managers`.`organization_id`
                     =
                     `organizations`.`id` )
                                  AND (
                     `organizations_billing_managers`.`user_id` =
                     1924 ) )
                            ) AS `t1`
                     UNION
                     SELECT `organizations`.`id`
                     FROM   `organizations`
                            INNER JOIN `organizations_auditors`
                                    ON ( (
                            `organizations_auditors`.`organization_id` =
                                         `organizations`.`id` )
                                         AND (
                            `organizations_auditors`.`user_id` = 1924 ) )
                                              ) AS
                                              `t1`) ) )
         AND ( `type` IN ( 'space_developer' ) )
         AND ( `user_id` IN (SELECT `id`
                             FROM   `users`
                             WHERE  ( `guid` IN (
                                      '26c20682-0ec5-4e13-817d-f64d6f580563'
                                                ) )) )
         AND ( `space_id` IN (SELECT `id`
                              FROM   `spaces`
                              WHERE  ( `guid` IN (
                                       '90775432-527f-4771-a1cc-17a4751de041'
                                                 ) )
                                 ) ) )
ORDER  BY `t1`.`created_at` ASC,
          `t1`.`guid` ASC
LIMIT  50 offset 0 

is this part of the Role model craziness? https://github.com/cloudfoundry/cloud_controller_ng/blob/4bc537c9ff2b01c46c2bdcfa4de480198c954875/app/models/runtime/role.rb#L8

@pusherofbrooms
Copy link
Author

That query makes my head hurt.

@philippthun
Copy link
Member

When I had a look at the index method of the RolesController, I've stumbled across this line. Although this role guid population should result in a no-op, it creates a transaction for each call to /v3/roles and thus could be causing problems when "multiple of them stack up". Because otherwise we are talking about "simple" SELECT statements that should be fast...

@sethboyles I think this RoleGuidPopulate class could simply be removed (according to the commit message this was only needed during a specific CAPI update); maybe we can call it once in a db migration step - just to be absolutely safe...

@philippthun
Copy link
Member

The query is a result of the "role model craziness" plus two "user visibility filters" (i.e. permission checks) - one for spaces: WHERE 'space_id' IN ..., one for orgs: WHERE 'organization_id' IN ....

@sethboyles
Copy link
Member

sethboyles commented Oct 21, 2022

Yeah, I agree that it's probably safe to remove the RoleGuidPopulate invocation now.

@pusherofbrooms
Copy link
Author

FYI, we worked around this issue with some database machinations. We currently use mysql v5.7.38 in RDS. 5.7 seems to have an in-memory temp table limit of 16M. I was noticing that all of the role queries temp tables were being moved to disk, so we set our temp table limit to 128M. This seems to have beneficial impact over all, and especially on these roles queries.

The settings we changed were tmp_table_size and max_heap_table_size, both to 128M. If someone else comes across this as a possible solution, be sure to check your freeable memory against your queries to determine if you have enough memory for this change.

Do the Cloud Foundry developers use mysql v5.8? It seems 5.8 has a different pooled mechanism for temp table limits such that you may have never run into this problem.

@sethboyles
Copy link
Member

We're seeing issues with this endpoint as well, and but changing tmp_table_size and max_heap_table_size didn't help in our case.

Some random findings, not on this endpoint specifically but using the Role, SpaceRole, and OrganizationRole models.

It seems the UNION ALL queries are particularly inefficient when applying a WHERE clause after generating the union of all the tables VS including a WHERE clause on each separate UNION. Example:

Current query:

mysql> SELECT 1 AS `one` FROM (SELECT 'space_developer' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_developers` UNION ALL SELECT 'sp
ace_auditor' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_auditors` UNION ALL SELECT 'space_supporter' AS `type`, `role_guid` AS `gui
d`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_supporters` UNION ALL SELECT 'space_manager' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `sp
ace_id`, `created_at`, `updated_at` FROM `spaces_managers`) AS `t1` WHERE ((`type` IN ('space_developer', 'space_manager', 'space_auditor', 'space_supporter')) AND (`user_id` = 3) AND (`space_id` IN (SELECT `id
` FROM `spaces` WHERE (`guid` = '300fd0a0-bb53-47b7-902e-b05f1085237f')))) LIMIT 1;
+-----+
| one |
+-----+
|   1 |
+-----+
1 row in set (1.25 sec)

Including separate WHERE clauses:

mysql> SELECT 1 AS `one` FROM (SELECT 'space_developer' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_developers` WHERE (`space_id` IN (SELECT `id` FROM `spaces` WHERE (`guid` = '300fd0a0-bb53-47b7-902e-b05f1085237f'))) UNION ALL SELECT 'space_auditor' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_auditors` WHERE (`space_id` IN (SELECT `id` FROM `spaces` WHERE (`guid` = '300fd0a0-bb53-47b7-902e-b05f1085237f'))) UNION ALL SELECT 'space_supporter' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_supporters` WHERE (`space_id` IN (SELECT `id` FROM `spaces` WHERE (`guid` = '300fd0a0-bb53-47b7-902e-b05f1085237f'))) UNION ALL SELECT 'space_manager' AS `type`, `role_guid` AS `guid`, `user_id`, -1 AS `organization_id`, `space_id`, `created_at`, `updated_at` FROM `spaces_managers` WHERE (`space_id` IN (SELECT `id` FROM `spaces` WHERE (`guid` = '300fd0a0-bb53-47b7-902e-b05f1085237f')))) AS `t1` WHERE ((`type` IN ('space_developer', 'space_manager', 'space_auditor', 'space_supporter')) AND (`user_id` = 3)) LIMIT 1;
+-----+
| one |
+-----+
|   1 |
+-----+
1 row in set (0.00 sec)

Although this query will be removed entirely with cloudfoundry/cloud_controller_ng#3043, I believe this general rule will apply to the v3/roles endpoint--if we could somehow apply the where filters before we generate the full unioned set, it should be more performant. I'm not sure how to do this with the Sequel ORM, other than dynamically generating Model classes with Class.new or something, which probably would have unexpected effects.

We could bypass Sequel entirely for this query, and fill out our own models from a SQL query. It could just be a simple Ruby object we instantiate to pass to the presenters.

Another idea is to migrate the individual role tables (e.g. organizations_managers, spaces_supporters, etc) to a single real roles table. This would also be very complicated and we would need to think about how to handle upgrades and we probably would have to update all the v2 code as well rewrite a lot v3 code.

@philippthun
Copy link
Member

I've opened a draft PR (cloudfoundry/cloud_controller_ng#3068) that moves most of the WHERE conditions into the individual SELECTs. Please head over to the PR/commit description for details.

When following this path we might also get rid of OrganizationRole and SpaceRole as this customized Role model/dataset contains a similar optimization.

@philippthun
Copy link
Member

I'm going to close this issue as the Optimize Role model/dataset PR has been merged and is released as part of CAPI 1.144.0. @pusherofbrooms - you should see a significant performance improvement with the latest CAPI version.

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

No branches or pull requests

4 participants