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

fix: high db cpu utilisation #3260

Merged
merged 6 commits into from
Oct 26, 2022
Merged

fix: high db cpu utilisation #3260

merged 6 commits into from
Oct 26, 2022

Conversation

arjprd
Copy link
Contributor

@arjprd arjprd commented Sep 16, 2022

While performing load test on new release of hydra found that on the DB side there is CPU crunch (100% for load of 30 tps on AWS RDS db.t3.xlarge) and from logs figured out this was caused by specific query
SELECT hydra_oauth2_flow.acr, hydra_oauth2_flow.amr, hydra_oauth2_flow.client_id, hydra_oauth2_flow.consent_challenge_id, hydra_oauth2_flow.consent_csrf, hydra_oauth2_flow.consent_error, hydra_oauth2_flow.consent_handled_at, hydra_oauth2_flow.consent_remember, hydra_oauth2_flow.consent_remember_for, hydra_oauth2_flow.consent_skip, hydra_oauth2_flow.consent_verifier, hydra_oauth2_flow.consent_was_used, hydra_oauth2_flow.context, hydra_oauth2_flow.forced_subject_identifier, hydra_oauth2_flow.granted_at_audience, hydra_oauth2_flow.granted_scope, hydra_oauth2_flow.login_authenticated_at, hydra_oauth2_flow.login_challenge, hydra_oauth2_flow.login_csrf, hydra_oauth2_flow.login_error, hydra_oauth2_flow.login_initialized_at, hydra_oauth2_flow.login_remember, hydra_oauth2_flow.login_remember_for, hydra_oauth2_flow.login_session_id, hydra_oauth2_flow.login_skip, hydra_oauth2_flow.login_verifier, hydra_oauth2_flow.login_was_used, hydra_oauth2_flow.nid, hydra_oauth2_flow.oidc_context, hydra_oauth2_flow.request_url, hydra_oauth2_flow.requested_at, hydra_oauth2_flow.requested_at_audience, hydra_oauth2_flow.requested_scope, hydra_oauth2_flow.session_access_token, hydra_oauth2_flow.session_id_token, hydra_oauth2_flow.state, hydra_oauth2_flow.subject FROM hydra_oauth2_flow AS hydra_oauth2_flow WHERE (state = 6 OR state = 5) AND subject = $1 AND client_id = $2 AND consent_skip=FALSE AND consent_error='{}' AND consent_remember=TRUE AND nid = $3 ORDER BY requested_at DESC LIMIT 1

So created a new composite index with all columns involved in the query lookup. The CPU utilization post introduction of index never exceeded 50% for load of 50+ tps

Related issue(s)

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

@CLAassistant
Copy link

CLAassistant commented Sep 16, 2022

CLA assistant check
All committers have signed the CLA.

@arjprd arjprd changed the title High DB CPU utilisation fix: High DB CPU utilisation Sep 16, 2022
@arjprd arjprd changed the title fix: High DB CPU utilisation fix: high db cpu utilisation Sep 16, 2022
@codecov
Copy link

codecov bot commented Sep 16, 2022

Codecov Report

Merging #3260 (8b77f5a) into master (8b77f5a) will not change coverage.
The diff coverage is n/a.

❗ Current head 8b77f5a differs from pull request most recent head b069c0d. Consider uploading reports for the commit b069c0d to get more accurate results

@@           Coverage Diff           @@
##           master    #3260   +/-   ##
=======================================
  Coverage   76.84%   76.84%           
=======================================
  Files         123      123           
  Lines        8912     8912           
=======================================
  Hits         6848     6848           
  Misses       1636     1636           
  Partials      428      428           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff, thank you! Could you please add migrations for the other SQL languages as well? :)

aeneasr
aeneasr previously approved these changes Oct 20, 2022
# Conflicts:
#	client/handler.go
#	consent/handler.go
#	health/doc.go
#	internal/httpclient/api_v0alpha2.go
#	internal/httpclient/api_wellknown.go
#	jwk/handler.go
#	oauth2/handler.go
#	oauth2/trust/handler.go
@aeneasr aeneasr merged commit 4bf995d into ory:master Oct 26, 2022
@vinckr
Copy link
Member

vinckr commented Oct 28, 2022

Hello @arjprd
Congrats on merging your first PR in Ory 🎉 !
Your contribution will soon be helping secure millions of identities around the globe 🌏.
As a small token of appreciation we send all our first time contributors a gift package to welcome them to the community.
Please drop me an email and I will forward you the form to claim your Ory swag!

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.

4 participants