Skip to content

Avoid duplicated hot-path User and ServiceProvider queries#6058

Merged
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/save-a-query-2
Mar 14, 2022
Merged

Avoid duplicated hot-path User and ServiceProvider queries#6058
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/save-a-query-2

Conversation

@mitchellhenke
Copy link
Contributor

User.find and ServiceProvider.find are two of our most frequent queries. Though they're fast, they make up a significant portion of total Postgres time

Part of that is due to querying for a ServiceProvider by issuer and User by id when adding SP cost. This PR lifts the queries out of Db::SpCost::AddSpCost and puts the responsibility on callers of the function to provide all of the data ahead of time. For the most part, we are querying the issuer in sp_session, which current_sp has probably already done.

It gets a little weird in doc auth with having to adjust the delegation and moving current_sp to a public method, but I think it's worth it.

There's opportunity to apply a similar pattern to IdentityLinker which accepts an issuer, but that can be saved for a followup if we're happy with the kind of adjustments being made here.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM! Nice finds

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/save-a-query-2 branch from d57ede3 to 135bb60 Compare March 14, 2022 20:47
Mitchell Henke and others added 2 commits March 14, 2022 15:48
changelog: Internal, Performance, Re-use existing database query results to avoid duplicative work
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/save-a-query-2 branch from 135bb60 to d53dc64 Compare March 14, 2022 20:48
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.

2 participants