Timeouts in include queries when using resource inheritance #1731
Labels
Comments
|
Let me provide some more Context. JsonApiDotNetCore: 5.6.0 Query String of Api Call: filter=and(any(id,'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX','XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX','XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX',XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX','XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX'),equals(isDeleted,'false'))&include=products.unitGroup.units Execution Time in Postman: 2.1s Logs: After Version Update without changing anything else... Logs: Please Notice that even EF Core has now provided new warnings after the update and If I were to show you the full query you could see that it trippled in its size... Since there is no EF Core update that our team did can you help us explain this? |
|
Are you using resource inheritance? It would help to share the before/after SQL for the request URL. |
|
Yes we are. products is a base resource with 5 or 6 children Resources |
|
@bkoelman Here is the generated Query in version 5.6.0: Here is the generate query in version 5.7.1: |
|
That looks awful :) Possibly caused by #1641. Can you try turning off pagination in options and see if that helps? Another thing to inspect (with pagination enabled) would be to compare |
|
@bkoelman Tried to apply your idea of turning off Pagination by setting DefaultPageSize = null, which wouldn't apply much in our scenarios since we are using this api to fill client Datagrids that use pagination. The change did affect the query but the end result is the same, in other words still produces a Timeout... Also here is the QueryLayer.ToString() when pagination is not set to null but also is not specifically set in the query string. It appears to be correct in my opinion and similar to logs we were originally getting and we sent in previous messages |
|
@bkoelman since it appears to be a bug of 5.7.0 and 5.7.1 versions may I suggest turning this issue into a bug and handling it this was? We have paused our version update until this is resolved. Please let us know if we can provide any further insight or assistance on resolving this since this is block us as well. |
|
I'm surprised by that query layer (assuming it originates from JADNC v5.7.1). I would have expected to see multiple field selectors (one per derived type), with pagination inside them. But it also has no includes, so it doesn't seem to represent the original problematic request with both a filter and includes. The initial log file contains "HTTP GET /v1/priceGroups", which doesn't have query string parameters, but they may have been trimmed off in logs for redaction. Can you confirm that the I don't know yet whether this is a bug or intended behavior. That doesn't mean I won't try to help you, but I need to understand the root cause first. Based on that, maybe JADNC can provide extra configuration settings to prevent this from happening, assuming the query becomes too complex for SQL Server to handle. But I'm just guessing now. It's hard for me to track what's happening, ie. I don't know the data model and its inheritance hierarchy. Would it be possible to create a minimal repro that I can debug, in the form of a git repo? Then I can analyze the query layer, the resulting LINQ expression, and its SQL, and compare those with the old version of JADNC. I also wonder if the problem is specific to SQL Server. Seeding the minimal repro with sample data should be easy with Bogus Faker, which we already use for integration tests.
Based on the conversations at dotnet/efcore#29182 and dotnet/efcore#29665, this warning may not be appropriate; the "solution" to activate query splitting may actually make the situation worse. I recommend ignoring this warning while trying to identify the root cause of the problem. |
|
Well, I suspect the printed I've merged a PR that logs both See the instructions at #1732 for how to activate it. It should become available shortly for download using https://github.com/json-api-dotnet/JsonApiDotNetCore?tab=readme-ov-file#trying-out-the-latest-build. |
|
@bkoelman here is a dummy repo that demonstrates issue: https://github.com/JohnStrim/JsonApiBugReport/tree/master |
|
@JohnStrim Thanks for the great repro project. It's been very helpful to further analyze what is happening. The PR at JohnStrim/JsonApiBugReport#1 contains a workaround. In case it gets lost, it's listed below. The slow queries are caused by a bugfix in JsonApiDotNetCore v5.7.0. In earlier versions, pagination on nested resources was not applied when resource inheritance was used. As a result of this fix, the query has become more complex, resulting in database timeouts. The sample project shows similar issues on both SQL Server and PostgreSQL. The query plans show lots of merges, but no full table scans. I don't know yet how exactly to improve the overall experience in JsonApiDotNetCore itself. The This is a tough problem to solve, and it's going to take some time to get this fully resolved. Until then, the Code to workaround slow queries// Register in your Program.cs with:
// builder.Services.AddTransient<IQueryableBuilder, PruningQueryableBuilder>();
using System;
using System.Linq;
using System.Linq.Expressions;
//using AgileObjects.ReadableExpressions;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Queries;
using JsonApiDotNetCore.Queries.QueryableBuilding;
using Microsoft.Extensions.Logging;
namespace JsonApiBugReport;
public class PruningQueryableBuilder(
IIncludeClauseBuilder includeClauseBuilder,
IWhereClauseBuilder whereClauseBuilder,
IOrderClauseBuilder orderClauseBuilder,
ISkipTakeClauseBuilder skipTakeClauseBuilder,
ISelectClauseBuilder selectClauseBuilder,
IJsonApiOptions options,
ILogger<PruningQueryableBuilder> logger)
: QueryableBuilder(includeClauseBuilder, whereClauseBuilder, orderClauseBuilder, skipTakeClauseBuilder,
selectClauseBuilder)
{
public override Expression ApplyQuery(QueryLayer layer, QueryableBuilderContext context)
{
ArgumentNullException.ThrowIfNull(layer);
Prune(layer);
var expression = base.ApplyQuery(layer, context);
// Uncomment the lines below to log the pruned query.
// It requires a NuGet reference to AgileObjects.ReadableExpressions.
//var text = expression.ToReadableString();
//if (text.StartsWith("[Microsoft.EntityFrameworkCore.Query.EntityQueryRootExpression]"))
//{
// logger.LogInformation("Expression (after prune): {Expression}", text);
//}
return expression;
}
private void Prune(QueryLayer queryLayer)
{
if (queryLayer.Selection != null)
{
foreach (var resourceType in queryLayer.Selection.GetResourceTypes().ToArray())
{
var selectors = queryLayer.Selection.GetOrCreateSelectors(resourceType);
foreach (var (field, subLayer) in selectors)
{
if (subLayer != null)
{
Prune(subLayer);
if (IsDefault(subLayer))
{
selectors.Remove(field);
}
}
}
}
if (queryLayer.Selection.IsEmpty)
{
queryLayer.Selection = null;
}
}
}
private bool IsDefault(QueryLayer queryLayer)
{
bool hasDefaultSort = queryLayer.Sort != null && queryLayer.Sort.ToString() == "id";
bool hasDefaultPagination = queryLayer.Pagination != null &&
Equals(queryLayer.Pagination.PageNumber, PageNumber.ValueOne) &&
Equals(queryLayer.Pagination.PageSize, options.DefaultPageSize);
var isDefault = queryLayer.Include == null && queryLayer.Filter == null &&
(queryLayer.Sort == null || hasDefaultSort) &&
(queryLayer.Pagination == null || hasDefaultPagination) && queryLayer.Selection == null;
return isDefault;
}
} |
bkoelman
changed the title
|
Even though what you propose seem to be working we are facing an issue with GetSecondary Endpoints. They always return: The reason behind this is that it finds that no sorting or pagination is being inserted and simply removes the applied query layer |
|
@bkoelman this minor edit to the Prune behavior appears to be solving the issue. Can also please check if what I'm doing is valid? |
4 tasks
|
Hard to say, but I doubt it's much better than what I came up with, which wasn't great either. The good news is I found a better way. See JohnStrim/JsonApiBugReport#2 and the description at #1735. Could you please try that and let me know how it works? |
|
@bkoelman we tried to install the latest prerelease (5.7.2-master-01204) in our Project. We still appear to face the same problem.
|
|
@bkoelman neither my change in the Pruning Behavior appears to be working. |
|
The pruner doesn't exist anymore. Did you follow the instructions described in JohnStrim/JsonApiBugReport#2, ie add resource definitions to your project? |
|
@bkoelman no you are correct, didn't do it this way. The problem is that we can't use it in this way (without pagination) because we are using pagination extensively in our api and unit and unitgroups have their own grids which utilize pagination. |
|
Can you sort them in de serialize callback on resource definitions? |
|
@bkoelman Ι wouldn't like to handle this logic in on deserialize. The reason is that some tables may contain thousands of data and unfortunately Db table are not in the best state possible (meaning that they don't have optimized indexes and have quite a lot of columns) so applying those queries without pagination and then sorting everything on application layer might cause huge delays which I would like to avoid. This is why unfortunately this is not an acceptable solution. |
|
@bkoelman I would like to try and better understand the problem with pagination in case I can provide some sort of help. |
|
Please read up on what I wrote in the comments of both PRs, I don't think you understand what's going on. The goal is to disable pagination only on small tables (unit groups and units) to simplify the SQL. The pruner also did that, but unreliable. As explained in the PRs, there is no other way to simplify the SQL. It is equivalent to older versions of JADNC, which mistakenly didn't add pagination in the first place. With the recent fixes, you now need to express where to turn off pagination, so the SQL remains simple. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
SUMMARY
Update to this version has produced many Timeouts in EF Core
DETAILS
In our logic we are extensively using query strings for filtering, including relationships and paging which in previous versions used to perform very well (Generally under 1 sec).
When updating to version 5.7.1 we started having a lot of Timeout in Db Queries. To provide more context we were trying to include relationships that are 2 or 3 level deep from our original resource: (ex. include=products.unitGroup,products.unitGroup.units)
Also we noticed a big delay even in simple GET queries which used to return result under 200ms and now performance has up to 600ms with no appart explanation.
We've tried to do an EF Performance Update: (Include the UseQuerySplittingBehavior(QuerySplittingBehavior.SplitQuery)) but this seems to causes many Data is Null Exceptions when including relationships.
Is there any suggestions as far as configuration or minimum versions that we should be aware of to avoid this overhead?
VERSIONS USED
The text was updated successfully, but these errors were encountered: