Skip to content

Improve performance of loading OAuth apps and tokens

Drew Blessing requested to merge dblessing_oauth_tokens_perf into master

What does this MR do and why?

Describe in detail what your merge request does and why.

Fixes #364827 (closed)

Resolves an N+1 and also optimizes how this controller and view work. Before we were finding tokens, then finding the applications from those tokens, then once again querying for the tokens associated with that application. What's more, we were also incorrectly scoping the lookup of tokens from applications and potentially showing inaccurate created at date/times.

This area of code will likely need to change again once !89854 (merged) is merged. I commented about this on !89854 (comment 999369746) and we may need to create a follow-up issue.

Database

Query:

SELECT "oauth_access_tokens".* FROM "oauth_access_tokens" WHERE "oauth_access_tokens"."resource_owner_id" = 1 AND "oauth_access_tokens"."revoked_at" IS NULL ORDER BY "oauth_access_tokens"."created_at" DESC

Migration

Migrate

main: == 20220621202616 AddPartialIndexOnOauthAccessTokensRevokedAt: migrating ======
main: -- transaction_open?()
main:    -> 0.0000s
main: -- index_exists?(:oauth_access_tokens, [:resource_owner_id, :created_at], {:name=>"partial_index_resource_owner_id_created_at_token_not_revoked", :where=>"revoked_at IS NULL", :algorithm=>:concurrently})
main:    -> 0.0034s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0002s
main: -- add_index(:oauth_access_tokens, [:resource_owner_id, :created_at], {:name=>"partial_index_resource_owner_id_created_at_token_not_revoked", :where=>"revoked_at IS NULL", :algorithm=>:concurrently})
main:    -> 0.0013s
main: -- execute("RESET statement_timeout")
main:    -> 0.0002s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- index_exists?(:oauth_access_tokens, :resource_owner_id, {:name=>"index_oauth_access_tokens_on_resource_owner_id", :algorithm=>:concurrently})
main:    -> 0.0016s
main: -- remove_index(:oauth_access_tokens, {:name=>"index_oauth_access_tokens_on_resource_owner_id", :algorithm=>:concurrently, :column=>:resource_owner_id})
main:    -> 0.0022s
main: == 20220621202616 AddPartialIndexOnOauthAccessTokensRevokedAt: migrated (0.0144s)

Rollback

main: == 20220621202616 AddPartialIndexOnOauthAccessTokensRevokedAt: reverting ======
main: -- transaction_open?()
main:    -> 0.0000s
main: -- index_exists?(:oauth_access_tokens, :resource_owner_id, {:name=>"index_oauth_access_tokens_on_resource_owner_id", :algorithm=>:concurrently})
main:    -> 0.0065s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0002s
main: -- add_index(:oauth_access_tokens, :resource_owner_id, {:name=>"index_oauth_access_tokens_on_resource_owner_id", :algorithm=>:concurrently})
main:    -> 0.0013s
main: -- execute("RESET statement_timeout")
main:    -> 0.0001s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- index_exists?(:oauth_access_tokens, [:resource_owner_id, :created_at], {:name=>"partial_index_resource_owner_id_created_at_token_not_revoked", :algorithm=>:concurrently})
main:    -> 0.0020s
main: -- remove_index(:oauth_access_tokens, {:name=>"partial_index_resource_owner_id_created_at_token_not_revoked", :algorithm=>:concurrently, :column=>[:resource_owner_id, :created_at]})
main:    -> 0.0025s
main: == 20220621202616 AddPartialIndexOnOauthAccessTokensRevokedAt: reverted (0.0196s)

Query Plans

Before index changes (this was on GitLab.com):

 Sort  (cost=83.23..83.26 rows=13 width=172) (actual time=188.258..188.268 rows=36 loops=1)
   Sort Key: oauth_access_tokens.created_at DESC
   Sort Method: quicksort  Memory: 34kB
   Buffers: shared hit=4 read=35
   I/O Timings: read=187.455 write=0.000
   ->  Index Scan using index_oauth_access_tokens_on_resource_owner_id on public.oauth_access_tokens  (cost=0.43..82.99 rows=13 width=172) (actual time=30.543..188.107 rows=36 loops=1)
         Index Cond: (oauth_access_tokens.resource_owner_id = 13356)
         Filter: (oauth_access_tokens.revoked_at IS NULL)
         Rows Removed by Filter: 1
         Buffers: shared hit=1 read=35
         I/O Timings: read=187.455 write=0.000

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Drew Blessing

Merge request reports

Loading