Table scans on idsrv.Tokens table when querying for expired tokens

[tristanbarcelon]

We are using EntityFramework persistence for IdentityServer3 tokens and I've noticed frequent queries for expired tokens against idsrv.Tokens that are in the form of:

exec sp_executesql N'SELECT 
[Extent1].[Key] AS [Key], 
[Extent1].[TokenType] AS [TokenType], 
[Extent1].[SubjectId] AS [SubjectId], 
[Extent1].[ClientId] AS [ClientId], 
[Extent1].[JsonCode] AS [JsonCode], 
[Extent1].[Expiry] AS [Expiry]
FROM [idsrv].[Tokens] AS [Extent1]
WHERE [Extent1].[Expiry] < @p__linq__0',N'@p__linq__0 datetimeoffset(7)',@p__linq__0='2016-02-04 23:25:31.2152527 +00:00'

There's currently no index that will satisfy this query so it performs a table scan instead. I'm proposing we change Token.cs to instead have a unique index on [Key] and [TokenType] columns and a non-unique clustered index on [Expiry] to help with the range query above. The updated Token class might look like the one below based on what I've read here: https://msdn.microsoft.com/en-us/data/jj591583 and https://msdn.microsoft.com/en-us/library/system.componentmodel.dataannotations.schema.indexattribute(v=vs.113).aspx.

public class Token
{
    [Index("UIX_idsrv_Tokens", 1)]
    public virtual string Key { get; set; }

    [Index("UIX_idsrv_Tokens", 2, IsUnique = True)]
    public virtual TokenType TokenType { get; set; }

    [StringLength(200)]
    public virtual string SubjectId { get; set; }

    [Required]
    [StringLength(200)]
    public virtual string ClientId { get; set; }

    [Required]
    [DataType(DataType.Text)]
    public virtual string JsonCode { get; set; }

    [Index("IXC_idsrv_Tokens", IsClustered = True)]
    [Required]
    public virtual DateTimeOffset Expiry { get; set; }
}

[brockallen]

Do you have a proposed change to the DbContext code add this index?

[feanz]

Just noticed this issue as well. The EF code created a query of tokens which is not needed the delete could be done without having to return the data. I think this is hard to do in EF. It make sense to just execute this as RAW SQL. If I get time soon I'll submit a PR for this.

[brockallen]

I think this comes from the code that looks for expired tokens. I'd ideally like a fix that can be configured in the DbContext's configure APIs (Build or whatever it's called), rather than something that's DB-specific (such as raw SQL).

[tristanbarcelon]

Would using something like EF Extended work? We use this package to avoid having to pull all entities back just to delete or update them.

https://github.com/loresoft/EntityFramework.Extended/wiki/Batch-Update-and-Delete

On Fri, May 13, 2016 at 8:24 AM, Brock Allen notifications@github.com wrote:

I think this comes from the code that looks for expired tokens. I'd ideally like a fix that can be configured in the DbContext's configure APIs (Build or whatever it's called), rather than something that's DB-specific (such as raw SQL).

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/IdentityServer/IdentityServer3.EntityFramework/issues/89#issuecomment-219028481

[brockallen]

Not sure -- what's the support on that? Does it target the same DBs that EF supports?

[tristanbarcelon]

EF Extended package supports the same DBs as EF and it has a dependency on EF >= 6.1.0.

https://www.nuget.org/packages/EntityFramework.Extended/

On Fri, May 13, 2016 at 8:38 AM, Brock Allen notifications@github.com wrote:

Not sure -- what's the support on that? Does it target the same DBs that EF supports?

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/IdentityServer/IdentityServer3.EntityFramework/issues/89#issuecomment-219031369

[brockallen]

Ok, possibly then. I don't know how much work it is to show me what it'd look like. Also, will this package allow indexes/unique constraints to be added?

[feanz]

Let me know what you think.

[HenrikWM]

Any news on when the 3.0.0 release with this fix will be released @brockallen ?

[brockallen]

After we get IdentityServer4 released.