[PM-3007] Caching user policies on PolicyService variable #3117
Conversation
|
I've placed the cached policies inside |
|
New Issues
|
CurrentContext is already doing too much in my opinion. I think it's much better to have this cached within the domain-specific service, which is exactly what you've done. I'm curious whether you've compared the query execution plans for the old & new sproc? I'm sure we're going to see some boost from the caching either way, so I'd like to know that we've also improved the sproc itself. If you want any help with this let me know. |
That is what I thought but I needed validation 😅
I did compare them! Here are both execution plans (old on top): 
I think its improved by having less nested loops but that my non-expert opinion. Let me know your thoughts. |
|
How'd you get these query plans by chance? You'll need the actual plans from production. |
That is true, who can I ping to get those? These are just from my local dev environment. |
|
Heh yeah I was asking the same questions to CloudOps and there wasn't an answer. For your sake I think you need to make an ad hoc request for them to jump in and pull some actual execution plans for you. |
|
@withinfocus I was aware of that limitation, but I thought using local query plans would be the next best thing. But comparing them, they're not even close, so that's good to know. I guess that means we also won't know of any measurable improvements until the new sproc has been running in prod for a while? |
|
In our situation that's pretty much correct. The usual course of action is to snapshot the table or simulate size and fragmentation so that it could be replicated elsewhere, but we are still adopting better database testing practices. |
It's worth checking qa cloud to see if that matches a little better, but this is probably true. |
…organization-user-read-by-user-id-with-policy-details-sproc # Conflicts: # src/Core/Repositories/IOrganizationUserRepository.cs
|
@kspearrin I experimented with adding two new indexes and changing an existing one to include some necessary columns. Here is the old plan when I ran the query locally: Here is the new plan with the indexes changes: With these changes I managed to reduce the amount of index scans but of course this is only my local environment so comparing the actual numbers is pointless. Do you think we should try to add the indexes? |
|
@r-tome I think those are good results, however, let's hold off on adding lots of indexes right now until we have the right people available to evaluate our indexing strategy. We can always add them on-demand if this query becomes more of a problem in production. |
| WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email | ||
| ) | ||
| END | ||
| GO |
| OR EXISTS ( | ||
| SELECT 1 | ||
| FROM [dbo].[UserView] U | ||
| WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email | ||
| ) | ||
| END |
r-tome
deleted the
PM-3007-resolve-high-db-resource-usage-by-organization-user-read-by-user-id-with-policy-details-sproc
branch
)" This reverts commit 78588d0.
Type of change
Objective
Enhance performance and reduce SQL operations by modifying the stored procedure
OrganizationUser_ReadByUserIdWithPolicyDetailsto retrieve all policies and filter them in memory for each type, thereby minimizing the number of SQL operations required. The filtered policies will be cached and utilized by the scoped serviceIPolicyService.Code changes
PolicyTypeso that the method returns all policy typesPolicyTypeso that the method returns all policy typesPolicyTypeso that the method returns all policy typesPolicyTypeso that the method returns all policy types and also replaced the nested query with left joinsBefore you submit
dotnet format --verify-no-changes) (required)