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

Cinstance N+1 issues clean-up #3889

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Commits on Sep 11, 2024

  1. Configuration menu
    Copy the full SHA
    c16f40d View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    8ed62c9 View commit details
    Browse the repository at this point in the history
  3. Bullet optimization of Admin::Api::ApplicationsController

    ```
    Bullet::Notification::UnoptimizedQueryError: user: localuser
    GET http://admin.foo.3scale.localhost:39073/p/admin/applications/16
    AVOID eager loading detected
      ApplicationPlan => [:plan_metrics]
      Remove from your query: .includes([:plan_metrics])
    ```
    akostadinov committed Sep 11, 2024
    Configuration menu
    Copy the full SHA
    f788d8d View commit details
    Browse the repository at this point in the history
  4. UnoptimizedQuery ApplicationPlan => [:original]

    Fixes issue with keys.feature:Regenerate provider key
    
    Bullet::Notification::UnoptimizedQueryError: user: avalon
    PUT http://master-account.3scale.localhost:42285/p/admin/applications/4/change_user_key
    AVOID eager loading detected
      ApplicationPlan => [:original]
      Remove from your query: .includes([:original])
    akostadinov committed Sep 11, 2024
    Configuration menu
    Copy the full SHA
    634e6ec View commit details
    Browse the repository at this point in the history
  5. debugging info on failure

    akostadinov committed Sep 11, 2024
    Configuration menu
    Copy the full SHA
    081a8c4 View commit details
    Browse the repository at this point in the history
  6. eager loading :default_application_plan of Service

    ```
    Bullet::Notification::UnoptimizedQueryError:
      user: default
      GET /admin/api/applications.json?provider_key=***&inactive_since=2014-05-05
      AVOID eager loading detected
        Service => [:default_application_plan]
        Remove from your query: .includes([:default_application_plan])
    ```
    akostadinov committed Sep 11, 2024
    Configuration menu
    Copy the full SHA
    37a6b21 View commit details
    Browse the repository at this point in the history
  7. fix user can access Cinstance

    The `#where_values_hash` method does not support joins and sub-queries.
    
    Originally the `account.provided_cinstances` part was ignored because it
    was JOINs. With the update to sub-queries, it turned into `plan_id: nil`
    which is incorrect and broke the logic.
    
    So now we keep logic as previously by resorting only to the
    `Cinstance.permitted_for` part of the query.
    
    This relies on the fact that `Cinstance.plan.issuer` is set as
    `Cinstance.service` when that issuer is a service.
    
    Also relies on the fact that `User.member_permission_service_ids` will
    not set to ids of services that don't belong to the account.
    
    Which may not be ideal but allows for permission checking without extra
    database queries.
    akostadinov committed Sep 11, 2024
    Configuration menu
    Copy the full SHA
    4e95548 View commit details
    Browse the repository at this point in the history
  8. access cinstance service directly not through plan

    Historically it was a conscious decision to optimize access to cintance
    -> service by denormalizing the database. So we can access service
    directly and not through the plan.
    
    We also keep these two in sync with model callbacks.
    
    So this commit further simplifies the queries to fully rely on this
    fact.
    akostadinov committed Sep 11, 2024
    Configuration menu
    Copy the full SHA
    ef6760b View commit details
    Browse the repository at this point in the history

Commits on Sep 13, 2024

  1. remove some n+1 issues

    akostadinov committed Sep 13, 2024
    Configuration menu
    Copy the full SHA
    aaea00d View commit details
    Browse the repository at this point in the history
  2. allow tracing SQL queries

    akostadinov committed Sep 13, 2024
    Configuration menu
    Copy the full SHA
    82e5492 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    01d602c View commit details
    Browse the repository at this point in the history
  4. N+1 in signup controller

    akostadinov committed Sep 13, 2024
    Configuration menu
    Copy the full SHA
    53d0222 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    62b07d9 View commit details
    Browse the repository at this point in the history