Identityserver4: PersistedGrants missing index on Expiration column

[chrklin]

Issue / Steps to reproduce the problem

According to DOC you can enable periodically cleanup of PersistedGrants.

But when looking at the class TokenCleanup we can see that it has a where clause on Expiration which does not have an index and the result will be a full table scan
Source Code Link

[brockallen]

Would changing this:

grant.HasIndex(x => new { x.SubjectId, x.ClientId, x.Type });

to this:

grant.HasIndex(x => new { x.SubjectId, x.ClientId, x.Type, x.Expiration });

be sufficient? In the past I've been told that adding a new index isn't that worthwhile, so just adding tot he existing one is sufficient.

[brockallen]

Also, this leads me to think we need this on the device table as well, and should add the device codes to the token cleanup too. // @scottbrady91

[scottbrady91]

Check out PR #3200 for cleanup. Agreed on the expiration

[brockallen]

Ok, i just went and tested with that index change and it seems like it's will use the index:

So I'll make those changes.

[brockallen]

Done.

[chrklin]

Hi @brockallen sorry i didn't follow up. I am testing it now. And I can see that you added the index last into an existing index, this will not have the desired effect.
As shown in your screen shot, it is in fact not using the index, it is doing a full table scan or full Clustered index scan on the primary key.
It is not using the index created with the Expiration Column.

[brockallen]

I added this in our 3.0 release.

[voroninp]

Is there any special purpose of selecting all the expired rows instead of just emitting raw sql to delete expired entries thus bypassing all EF's pipeline?

[brockallen]

Because we don't know what DB provider you're using. EF helps abstract that.

[voroninp]

Ok, then this feature should be requested on their side.

[brockallen]

Looks like this has been fixed already. Closing.

[lock[bot]]

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