EF Enhancement? #298

leastprivilege · 2021-06-21T09:30:28Z

There are 8 child tables with FK to Client Table - https://id4withclients.readthedocs.io/en/latest/_images/ClientAppRelatedTables.png

By using ASSplitQuery, this results in multiple queries without left join
https://docs.microsoft.com/en-us/ef/core/querying/single-split-queries?WT.mc_id=DOP-MVP-5001942#split-queries-1

Originally posted by @maulik-modi in https://github.com/DuendeSoftware/IdentityServer/discussions/293

maulik-modi · 2021-06-21T10:47:34Z

`2021-06-18 12:54:37.995 +00:00 [INF] Executed DbCommand (6ms) [Parameters=[@__clientId_0='?'], CommandType='"Text"', CommandTimeout='30']
SELECT c0."Id", c0."ClientId", c0."Type", c0."Value"
FROM "Clients" AS c
INNER JOIN "ClientClaims" AS c0 ON c."Id" = c0."ClientId"
WHERE c."ClientId" = @__clientId_0

2021-06-18 12:54:38.003 +00:00 [INF] Executed DbCommand (6ms) [Parameters=[@__clientId_0='?'], CommandType='"Text"', CommandTimeout='30']
SELECT c0."Id", c0."ClientId", c0."Created", c0."Description", c0."Expiration", c0."Type", c0."Value"
FROM "Clients" AS c
INNER JOIN "ClientSecrets" AS c0 ON c."Id" = c0."ClientId"
WHERE c."ClientId" = @__clientId_0

2021-06-18 12:54:38.009 +00:00 [INF] Executed DbCommand (6ms) [Parameters=[@__clientId_0='?'], CommandType='"Text"', CommandTimeout='30']
SELECT c0."Id", c0."ClientId", c0."Provider"
FROM "Clients" AS c
INNER JOIN "ClientIdPRestrictions" AS c0 ON c."Id" = c0."ClientId"
WHERE c."ClientId" = @__clientId_0

2021-06-18 12:54:38.018 +00:00 [INF] Executed DbCommand (8ms) [Parameters=[@__clientId_0='?'], CommandType='"Text"', CommandTimeout='30']
SELECT c0."Id", c0."ClientId", c0."PostLogoutRedirectUri"
FROM "Clients" AS c
INNER JOIN "ClientPostLogoutRedirectUris" AS c0 ON c."Id" = c0."ClientId"
WHERE c."ClientId" = @__clientId_0

2021-06-18 12:54:38.024 +00:00 [INF] Executed DbCommand (6ms) [Parameters=[@__clientId_0='?'], CommandType='"Text"', CommandTimeout='30']
SELECT c0."Id", c0."ClientId", c0."Key", c0."Value"
FROM "Clients" AS c
INNER JOIN "ClientProperties" AS c0 ON c."Id" = c0."ClientId"
WHERE c."ClientId" = @__clientId_0

2021-06-18 12:54:38.031 +00:00 [INF] Executed DbCommand (6ms) [Parameters=[@__clientId_0='?'], CommandType='"Text"', CommandTimeout='30']
SELECT c0."Id", c0."ClientId", c0."RedirectUri"
FROM "Clients" AS c
INNER JOIN "ClientRedirectUris" AS c0 ON c."Id" = c0."ClientId"
WHERE c."ClientId" = @__clientId_0`

hugoqribeiro · 2021-06-21T10:58:30Z

This is something that is generating a lot of warning on our setup.
I'm not sure that changing the behavior would improve performance, though... :)

maulik-modi · 2021-06-21T15:37:12Z

@hugoqribeiro , can you elaborate with necessary details?

maulik-modi · 2021-06-21T15:41:37Z

Current Implementation:
https://github.com/DuendeSoftware/IdentityServer/blob/main/src/EntityFramework.Storage/Stores/ClientStore.cs

stefannikolei · 2021-06-21T18:10:56Z

To support this, the support for .netcoreapp3.1 must be removed. This is probably a thing to look at in the .NET6.0 Time frame or?

hugoqribeiro · 2021-06-21T19:27:26Z

@hugoqribeiro , can you elaborate with necessary details?

We get this warning often:

Compiling a query which loads related collections for more than one collection navigation either via 'Include' or through projection but no 'QuerySplittingBehavior' has been configured. By default Entity Framework will use 'QuerySplittingBehavior.SingleQuery' which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call 'ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning))'

This comes from our own custom code (a back-office for Identity Server) where the DB context uses this:

return this.Clients
    .Include(x => x.AllowedCorsOrigins)
    .Include(x => x.AllowedGrantTypes)
    .Include(x => x.AllowedScopes)
    .Include(x => x.Claims)
    .Include(x => x.ClientSecrets)
    .Include(x => x.IdentityProviderRestrictions)
    .Include(x => x.PostLogoutRedirectUris)
    .Include(x => x.Properties)
    .Include(x => x.RedirectUris);

My question is: how is this worst than splitting the queries?

maulik-modi · 2021-06-22T04:10:30Z

@hugoqribeiro , depends on the number of clients you have.

maulik-modi · 2021-06-30T07:57:35Z

@stefannikolei and @brockallen EF Core 5.0.7 requires minimum version of .NET STandard 2.1;

Source: https://www.nuget.org/packages/Microsoft.EntityFrameworkCore/#:~:text=Entity%20Framework%20Core%20is%20a%20modern%20object-database%20mapper,provider%20plugin%20API.%20Requires%20NuGet%203.6%20or%20higher.

Source: https://docs.microsoft.com/en-us/ef/core/miscellaneous/platforms
Am I missing something when you say - " .netcoreapp3.1 must be removed"

Similar context: dotnet/efcore#23735

maulik-modi · 2021-06-30T08:11:44Z

Few more queries

2021-06-30 07:27:29.338 +00:00 [INF] Executed DbCommand (16ms) [Parameters=[@__names_0='?' (DbType = Object)], CommandType='"Text"', CommandTimeout='30']

SELECT a."Id", a."AllowedAccessTokenSigningAlgorithms", a."Created", a."Description", a."DisplayName", a."Enabled", a."LastAccessed", a."Name", a."NonEditable", a."RequireResourceIndicator", a."ShowInDiscoveryDocument", a."Updated", a0."Id", a0."ApiResourceId", a0."Created", a0."Description", a0."Expiration", a0."Type", a0."Value", a1."Id", a1."ApiResourceId", a1."Scope", a2."Id", a2."ApiResourceId", a2."Type", a3."Id", a3."ApiResourceId", a3."Key", a3."Value"
FROM "ApiResources" AS a
LEFT JOIN "ApiResourceSecrets" AS a0 ON a."Id" = a0."ApiResourceId"
LEFT JOIN "ApiResourceScopes" AS a1 ON a."Id" = a1."ApiResourceId"
LEFT JOIN "ApiResourceClaims" AS a2 ON a."Id" = a2."ApiResourceId"
LEFT JOIN "ApiResourceProperties" AS a3 ON a."Id" = a3."ApiResourceId"
WHERE EXISTS (
    SELECT 1
    FROM "ApiResourceScopes" AS a4
    WHERE (a."Id" = a4."ApiResourceId") AND a4."Scope" = ANY (@__names_0))
ORDER BY a."Id", a0."Id", a1."Id", a2."Id", a3."Id"

SELECT a."Id", a."Description", a."DisplayName", a."Emphasize", a."Enabled", a."Name", a."Required", a."ShowInDiscoveryDocument", a0."Id", a0."ScopeId", a0."Type", a1."Id", a1."Key", a1."ScopeId", a1."Value"
FROM "ApiScopes" AS a
LEFT JOIN "ApiScopeClaims" AS a0 ON a."Id" = a0."ScopeId"
LEFT JOIN "ApiScopeProperties" AS a1 ON a."Id" = a1."ScopeId"
WHERE a."Name" = ANY (@__scopes_0)
ORDER BY a."Id", a0."Id", a1."Id"

brockallen · 2021-09-02T13:31:34Z

Now that we're on .NET 6 (preview) I went back to investigate this issue (with SqlServer). Given how the client store is written, the queries for the client table are already split (albeit manually). The resource queries are not. Given how few columns the resource tables have (when compared to the client table), I'm not sure split queries buy us that much benefit -- anyone have any data on this in your running servers?

brockallen · 2021-09-02T14:37:35Z

I ran some tests with split query and without:

|              Method |     Mean |     Error |    StdDev | Ratio | RatioSD |
|-------------------- |---------:|----------:|----------:|------:|--------:|
|    SplitQueryClient | 2.564 ms | 0.1179 ms | 0.3439 ms |  1.00 |    0.00 |
| NotSplitQueryClient | 1.174 ms | 0.0323 ms | 0.0910 ms |  0.47 |    0.08 |

So in my quick and dirty local tests show that the split query is 2x slower than no split query. This makes me want to not use it by default. But the good news is that if you really do want it, then it can be configured when you register the DbContext in DI.

brockallen · 2021-09-02T14:39:18Z

The non-split query used was this:

SELECT [t].[Id], [t].[AbsoluteRefreshTokenLifetime], [t].[AccessTokenLifetime], [t].[AccessTokenType], [t].[AllowAccessTokensViaBrowser], [t].[AllowOfflineAccess], [t].[AllowPlainTextPkce], [t].[AllowRememberConsent], [t].[AllowedIdentityTokenSigningAlgorithms], [t].[AlwaysIncludeUserClaimsInIdToken], [t].[AlwaysSendClientClaims], [t].[AuthorizationCodeLifetime], [t].[BackChannelLogoutSessionRequired], [t].[BackChannelLogoutUri], [t].[ClientClaimsPrefix], [t].[ClientId], [t].[ClientName], [t].[ClientUri], [t].[ConsentLifetime], [t].[Created], [t].[Description], [t].[DeviceCodeLifetime], [t].[EnableLocalLogin], [t].[Enabled], [t].[FrontChannelLogoutSessionRequired], [t].[FrontChannelLogoutUri], [t].[IdentityTokenLifetime], [t].[IncludeJwtId], [t].[LastAccessed], [t].[LogoUri], [t].[NonEditable], [t].[PairWiseSubjectSalt], [t].[ProtocolType], [t].[RefreshTokenExpiration], [t].[RefreshTokenUsage], [t].[RequireClientSecret], [t].[RequireConsent], [t].[RequirePkce], [t].[RequireRequestObject], [t].[SlidingRefreshTokenLifetime], [t].[UpdateAccessTokenClaimsOnRefresh], [t].[Updated], [t].[UserCodeType], [t].[UserSsoLifetime], [c0].[Id], [c0].[ClientId], [c0].[Origin], [c1].[Id], [c1].[ClientId], [c1].[GrantType], [c2].[Id], [c2].[ClientId], [c2].[Scope], [c3].[Id], [c3].[ClientId], [c3].[Type], [c3].[Value], [c4].[Id], [c4].[ClientId], [c4].[Created], [c4].[Description], [c4].[Expiration], [c4].[Type], [c4].[Value], [c5].[Id], [c5].[ClientId], [c5].[Provider], [c6].[Id], [c6].[ClientId], [c6].[PostLogoutRedirectUri], [c7].[Id], [c7].[ClientId], [c7].[Key], [c7].[Value], [c8].[Id], [c8].[ClientId], [c8].[RedirectUri]
FROM (
    SELECT TOP(2) [c].[Id], [c].[AbsoluteRefreshTokenLifetime], [c].[AccessTokenLifetime], [c].[AccessTokenType], [c].[AllowAccessTokensViaBrowser], [c].[AllowOfflineAccess], [c].[AllowPlainTextPkce], [c].[AllowRememberConsent], [c].[AllowedIdentityTokenSigningAlgorithms], [c].[AlwaysIncludeUserClaimsInIdToken], [c].[AlwaysSendClientClaims], [c].[AuthorizationCodeLifetime], [c].[BackChannelLogoutSessionRequired], [c].[BackChannelLogoutUri], [c].[ClientClaimsPrefix], [c].[ClientId], [c].[ClientName], [c].[ClientUri], [c].[ConsentLifetime], [c].[Created], [c].[Description], [c].[DeviceCodeLifetime], [c].[EnableLocalLogin], [c].[Enabled], [c].[FrontChannelLogoutSessionRequired], [c].[FrontChannelLogoutUri], [c].[IdentityTokenLifetime], [c].[IncludeJwtId], [c].[LastAccessed], [c].[LogoUri], [c].[NonEditable], [c].[PairWiseSubjectSalt], [c].[ProtocolType], [c].[RefreshTokenExpiration], [c].[RefreshTokenUsage], [c].[RequireClientSecret], [c].[RequireConsent], [c].[RequirePkce], [c].[RequireRequestObject], [c].[SlidingRefreshTokenLifetime], [c].[UpdateAccessTokenClaimsOnRefresh], [c].[Updated], [c].[UserCodeType], [c].[UserSsoLifetime]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
LEFT JOIN [ClientCorsOrigins] AS [c0] ON [t].[Id] = [c0].[ClientId]
LEFT JOIN [ClientGrantTypes] AS [c1] ON [t].[Id] = [c1].[ClientId]
LEFT JOIN [ClientScopes] AS [c2] ON [t].[Id] = [c2].[ClientId]
LEFT JOIN [ClientClaims] AS [c3] ON [t].[Id] = [c3].[ClientId]
LEFT JOIN [ClientSecrets] AS [c4] ON [t].[Id] = [c4].[ClientId]
LEFT JOIN [ClientIdPRestrictions] AS [c5] ON [t].[Id] = [c5].[ClientId]
LEFT JOIN [ClientPostLogoutRedirectUris] AS [c6] ON [t].[Id] = [c6].[ClientId]
LEFT JOIN [ClientProperties] AS [c7] ON [t].[Id] = [c7].[ClientId]
LEFT JOIN [ClientRedirectUris] AS [c8] ON [t].[Id] = [c8].[ClientId]
ORDER BY [t].[Id], [c0].[Id], [c1].[Id], [c2].[Id], [c3].[Id], [c4].[Id], [c5].[Id], [c6].[Id], [c7].[Id], [c8].[Id]

and the split queries were these (of course, each one was a DB round trip, which explains the per difference):

SELECT TOP(2) [c].[Id], [c].[AbsoluteRefreshTokenLifetime], [c].[AccessTokenLifetime], [c].[AccessTokenType], [c].[AllowAccessTokensViaBrowser], [c].[AllowOfflineAccess], [c].[AllowPlainTextPkce], [c].[AllowRememberConsent], [c].[AllowedIdentityTokenSigningAlgorithms], [c].[AlwaysIncludeUserClaimsInIdToken], [c].[AlwaysSendClientClaims], [c].[AuthorizationCodeLifetime], [c].[BackChannelLogoutSessionRequired], [c].[BackChannelLogoutUri], [c].[ClientClaimsPrefix], [c].[ClientId], [c].[ClientName], [c].[ClientUri], [c].[ConsentLifetime], [c].[Created], [c].[Description], [c].[DeviceCodeLifetime], [c].[EnableLocalLogin], [c].[Enabled], [c].[FrontChannelLogoutSessionRequired], [c].[FrontChannelLogoutUri], [c].[IdentityTokenLifetime], [c].[IncludeJwtId], [c].[LastAccessed], [c].[LogoUri], [c].[NonEditable], [c].[PairWiseSubjectSalt], [c].[ProtocolType], [c].[RefreshTokenExpiration], [c].[RefreshTokenUsage], [c].[RequireClientSecret], [c].[RequireConsent], [c].[RequirePkce], [c].[RequireRequestObject], [c].[SlidingRefreshTokenLifetime], [c].[UpdateAccessTokenClaimsOnRefresh], [c].[Updated], [c].[UserCodeType], [c].[UserSsoLifetime]
FROM [Clients] AS [c]
WHERE [c].[ClientId] = N'client'
ORDER BY [c].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[Origin], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientCorsOrigins] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[GrantType], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientGrantTypes] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[Scope], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientScopes] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[Type], [c0].[Value], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientClaims] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[Created], [c0].[Description], [c0].[Expiration], [c0].[Type], [c0].[Value], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientSecrets] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[Provider], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientIdPRestrictions] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[PostLogoutRedirectUri], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientPostLogoutRedirectUris] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[Key], [c0].[Value], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientProperties] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go
SELECT [c0].[Id], [c0].[ClientId], [c0].[RedirectUri], [t].[Id]
FROM (
    SELECT TOP(1) [c].[Id]
    FROM [Clients] AS [c]
    WHERE [c].[ClientId] = N'client'
) AS [t]
INNER JOIN [ClientRedirectUris] AS [c0] ON [t].[Id] = [c0].[ClientId]
ORDER BY [t].[Id]
go

maulik-modi · 2021-09-02T14:46:17Z

@brockallen , you are right, there is significant latency.

Each query currently implies an additional network roundtrip to your database. Multiple network roundtrips can degrade performance, especially where latency to the database is high (for example, cloud services).

github-actions · 2021-09-18T00:17:31Z

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue.

leastprivilege assigned brockallen Jun 21, 2021

leastprivilege added the investigating label Jun 21, 2021

brockallen mentioned this issue Sep 2, 2021

[EF] Optimize client query to use a single DB round trip #388

Merged

brockallen added the state/wontfix This will not be worked on label Sep 3, 2021

brockallen closed this as completed Sep 3, 2021

This was referenced Sep 14, 2021

Ignore EF warning about split vs non-split queries #406

Merged

Suppress EF warning about split query behavior #409

Merged

github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EF Enhancement? #298

EF Enhancement? #298

leastprivilege commented Jun 21, 2021

maulik-modi commented Jun 21, 2021

hugoqribeiro commented Jun 21, 2021

maulik-modi commented Jun 21, 2021

maulik-modi commented Jun 21, 2021

stefannikolei commented Jun 21, 2021

hugoqribeiro commented Jun 21, 2021

maulik-modi commented Jun 22, 2021

maulik-modi commented Jun 30, 2021 •

edited

Loading

maulik-modi commented Jun 30, 2021

brockallen commented Sep 2, 2021 •

edited

Loading

brockallen commented Sep 2, 2021

brockallen commented Sep 2, 2021

maulik-modi commented Sep 2, 2021

github-actions bot commented Sep 18, 2021

EF Enhancement? #298

EF Enhancement? #298

Comments

leastprivilege commented Jun 21, 2021

maulik-modi commented Jun 21, 2021

hugoqribeiro commented Jun 21, 2021

maulik-modi commented Jun 21, 2021

maulik-modi commented Jun 21, 2021

stefannikolei commented Jun 21, 2021

hugoqribeiro commented Jun 21, 2021

maulik-modi commented Jun 22, 2021

maulik-modi commented Jun 30, 2021 • edited Loading

maulik-modi commented Jun 30, 2021

brockallen commented Sep 2, 2021 • edited Loading

brockallen commented Sep 2, 2021

brockallen commented Sep 2, 2021

maulik-modi commented Sep 2, 2021

github-actions bot commented Sep 18, 2021

maulik-modi commented Jun 30, 2021 •

edited

Loading

brockallen commented Sep 2, 2021 •

edited

Loading