Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Codecov Report

Merging #3115 (e30b4f2) into master (0a77a06) will decrease coverage by 0.18%.
The diff coverage is 83.78%.

❗ Current head e30b4f2 differs from pull request most recent head 13d762b. Consider uploading reports for the commit 13d762b to get more accurate results

@@            Coverage Diff             @@
##           master    #3115      +/-   ##
==========================================
- Coverage   79.67%   79.48%   -0.19%     
==========================================
  Files         112      112              
  Lines        7932     7864      -68     
==========================================
- Hits         6320     6251      -69     
  Misses       1214     1214              
- Partials      398      399       +1     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e586fd7...13d762b. Read the comment docs.

Hello,

I had a look on v2.x branch. It seems the code changed a lot.
The authentication and request tables has been merged in an unique flow tables, so I can't give you production feedback on this table as the v2 has not yet been released :(

However, the main issue still arise on refresh_token table even if the query changed (https://github.com/ory/hydra/blob/v2.x/persistence/sql/persister_oauth2.go#L430) :

• it is still missing an hydra_oauth2_refresh_requested_at_idx index on (nid, requested_at)
• and ORDER BY still using signature instead of requested_at

By the way, I think it would be better if the existing index on access_token table would be on (nid, requested_at), rather than on (requested_at, nid). With composite index, it's always better to put more selective index at first, that is, equality first, and then comparison.

I've made tests we a small amount of data, and forcing usage of indexes on PostgreSQL:

Here the actual query plan :
Limit (cost=12.27..12.28 rows=1 width=44) (actual time=0.048..0.049 rows=0 loops=1)
-> Sort (cost=12.27..12.28 rows=1 width=44) (actual time=0.048..0.048 rows=0 loops=1)
Sort Key: signature
Sort Method: quicksort Memory: 25kB
-> Index Scan using hydra_oauth2_refresh_client_id_idx on hydra_oauth2_refresh (cost=0.14..12.26 rows=1 width=44) (actual time=0.043..0.044 rows=0 loops=1)
Index Cond: (nid = '4511a21c-67e1-4a80-8999-444c1a418ce6'::uuid)
Filter: (requested_at < '2021-12-12 00:00:00'::timestamp without time zone)
Rows Removed by Filter: 1
Planning time: 0.105 ms
Execution time: 0.065 ms

With index on (nid, requested_at)
Limit (cost=8.16..8.17 rows=1 width=44) (actual time=0.015..0.015 rows=0 loops=1)
-> Sort (cost=8.16..8.17 rows=1 width=44) (actual time=0.014..0.015 rows=0 loops=1)
Sort Key: signature
Sort Method: quicksort Memory: 25kB
-> Index Scan using hydra_oauth2_refresh_requested_at_idx on hydra_oauth2_refresh (cost=0.14..8.15 rows=1 width=44) (actual time=0.009..0.009 rows=0 loops=1)
Index Cond: ((nid = '4511a21c-67e1-4a80-8999-444c1a418ce6'::uuid) AND (requested_at < '2021-12-12 00:00:00'::timestamp without time zone))
Planning time: 0.101 ms
Execution time: 0.034 ms

With index on (nid, requested_at) and ORDER BY requested_at, we have no sorting, no full scan, and we have full usage of index. :
Limit (cost=0.14..8.15 rows=1 width=52) (actual time=0.011..0.011 rows=0 loops=1)
-> Index Scan using hydra_oauth2_refresh_requested_at_idx on hydra_oauth2_refresh (cost=0.14..8.15 rows=1 width=52) (actual time=0.010..0.010 rows=0 loops=1)
Index Cond: ((nid = '4511a21c-67e1-4a80-8999-444c1a418ce6'::uuid) AND (requested_at < '2021-12-12 00:00:00'::timestamp without time zone))
Planning time: 0.124 ms
Execution time: 0.036 ms

Since the code change a lot, I think it would be better to create a new pull request, or an issue (I don't know yet if I would have time to create a new pull request).

Superseded by #3516