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

ID-891 Optimize UserResourcesQuery #1226

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

Ghost-in-a-Jar
Copy link
Contributor

@Ghost-in-a-Jar Ghost-in-a-Jar commented Oct 25, 2023

Ticket: https://broadworkbench.atlassian.net/browse/ID-891

What/why:

This query is accounting for the majority of the cpu load in sam, for some reason postgres is doing a full table scan of sam_effective_resource_policy every single time the query is ran, despite there being indexes on the table. This modification forces the join of sam_effective_resource_policy to filter down to only resource_ids of the type we are looking for.

How:

This modification to the query should be functionally equivalent, but forces the postgres query planner to do an index scan instead of a full table scan:

before
-> Parallel Seq Scan on sam_effective_resource_policy effpol (cost=0.00..14559.75 rows=537375 width=24)
after
-> Index Scan using idx_erp_resource_id on sam_effective_resource_policy effpol (cost=0.43..0.63 rows=6 width=24)


PR checklist

  • I've followed the instructions if I've made any changes to the API, especially if they're breaking changes
  • I've filled out the Security Risk Assessment (requires Broad Internal network access) and attached the result to the JIRA ticket

Copy link
Collaborator

@dvoet dvoet left a comment

Choose a reason for hiding this comment

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

made a suggestion for now or for the future

Comment on lines +1786 to +1790
join ${ResourceTypeTable as resourceType} on ${resourceType.id} = ${resource.resourceTypeId}
where ${resourceType.name} = ${resourceTypeName}
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you want to go a little further

Suggested change
join ${ResourceTypeTable as resourceType} on ${resourceType.id} = ${resource.resourceTypeId}
where ${resourceType.name} = ${resourceTypeName}
where ${resource.resourceTypeId} = ${resourceTypePKsByName(resourceTypeName)}

also could remove the ResourceTypeTable join and replace

where ${resourceType.name} = ${resourceTypeName}

with

where ${resource.resourceTypeId} = ${resourceTypePKsByName(resourceTypeName)}

Copy link
Contributor

@tlangs tlangs left a comment

Choose a reason for hiding this comment

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

Lets test on Dev and make sure that the query is returning everything and behaving as we expect. Once that's done, to prod we go!

@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-891-optimize-user-resources-query branch from 14ab202 to 0f342b3 Compare October 25, 2023 14:17
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Ghost-in-a-Jar
Copy link
Contributor Author

@tlangs I ran:

(select * from optimized EXCEPT select * from original)
UNION ALL
(select * from original EXCEPT select * from optimized)

and that returned nothing so they are returning the same rows

@Ghost-in-a-Jar Ghost-in-a-Jar merged commit d18d97b into develop Oct 25, 2023
15 checks passed
@Ghost-in-a-Jar Ghost-in-a-Jar deleted the ID-891-optimize-user-resources-query branch October 25, 2023 14:34
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.

3 participants