Skip to content

Commit

Permalink
Improve performance by optimizing database queries (#996)
Browse files Browse the repository at this point in the history
* fix: added a mitigation for the single-query ef-core warning

* fix: re-added IncludeAll

* fix: added a mitigation for the single-query ef-core warning - part II

* fix: done

* chore: reformat code

* chore: removed unused namespace

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
ericbrunner and mergify[bot] authored Dec 18, 2024
1 parent 86a491b commit 0d73e16
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Backbone.DevelopmentKit.Identity.ValueObjects;
using Backbone.Tooling.Extensions;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.Extensions.DependencyInjection;
using Npgsql.EntityFrameworkCore.PostgreSQL;
Expand Down Expand Up @@ -83,12 +84,28 @@ public async Task<T> RunInTransaction<T>(Func<Task<T>> func, IsolationLevel isol
return await RunInTransaction(func, null, isolationLevel);
}

public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = new())
{
var entities = GetChangedEntities();
var result = base.SaveChangesAsync(cancellationToken);
PublishDomainEvents(entities);

return result;
}

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
base.OnConfiguring(optionsBuilder);

if (EnvironmentVariables.DEBUG_PERFORMANCE && _serviceProvider != null)
optionsBuilder.AddInterceptors(_serviceProvider.GetRequiredService<SaveChangesTimeInterceptor>());

#if DEBUG
// Note: That option raises an exception when multiple collections are included in a query. It should help while debugging to
// find out where the issue is. In case of such exception you should use the .AsSplitQuery() method to split the query into
// multiple queries. See: https://learn.microsoft.com/en-us/ef/core/querying/single-split-queries#split-queries
optionsBuilder.ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning));
#endif
}

protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
Expand Down Expand Up @@ -121,15 +138,6 @@ public override int SaveChanges(bool acceptAllChangesOnSuccess)
return result;
}

public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = new())
{
var entities = GetChangedEntities();
var result = base.SaveChangesAsync(cancellationToken);
PublishDomainEvents(entities);

return result;
}

public override Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = new())
{
var entities = GetChangedEntities();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Backbone.Tooling.Extensions;
using Microsoft.AspNetCore.Identity.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.Extensions.DependencyInjection;
using Npgsql.EntityFrameworkCore.PostgreSQL;
Expand All @@ -27,9 +28,9 @@ public class DevicesDbContext : IdentityDbContext<ApplicationUser>, IDevicesDbCo
private const string SQLSERVER = "Microsoft.EntityFrameworkCore.SqlServer";
private const string POSTGRES = "Npgsql.EntityFrameworkCore.PostgreSQL";
private static readonly TimeSpan MAX_RETRY_DELAY = TimeSpan.FromSeconds(1);
private readonly IEventBus _eventBus;

private readonly IServiceProvider? _serviceProvider;
private readonly IEventBus _eventBus;

public DevicesDbContext(DbContextOptions<DevicesDbContext> options) : base(options)
{
Expand Down Expand Up @@ -65,14 +66,6 @@ public IQueryable<T> SetReadOnly<T>() where T : class
return Set<T>().AsNoTracking();
}

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
base.OnConfiguring(optionsBuilder);

if (EnvironmentVariables.DEBUG_PERFORMANCE && _serviceProvider != null)
optionsBuilder.AddInterceptors(_serviceProvider.GetRequiredService<SaveChangesTimeInterceptor>());
}

public async Task RunInTransaction(Func<Task> action, List<int>? errorNumbersToRetry,
IsolationLevel isolationLevel = IsolationLevel.ReadCommitted)
{
Expand Down Expand Up @@ -119,6 +112,31 @@ public async Task<T> RunInTransaction<T>(Func<Task<T>> func, IsolationLevel isol
return await RunInTransaction(func, null, isolationLevel);
}

public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = new())
{
var entities = GetChangedEntities();
var result = base.SaveChangesAsync(cancellationToken);
PublishDomainEvents(entities);

return result;
}

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
base.OnConfiguring(optionsBuilder);

if (EnvironmentVariables.DEBUG_PERFORMANCE && _serviceProvider != null)
optionsBuilder.AddInterceptors(_serviceProvider.GetRequiredService<SaveChangesTimeInterceptor>());


#if DEBUG
// Note: That option raises an exception when multiple collections are included in a query. It should help while debugging to
// find out where the issue is. In case of such exception you should use the .AsSplitQuery() method to split the query into
// multiple queries. See: https://learn.microsoft.com/en-us/ef/core/querying/single-split-queries#split-queries
optionsBuilder.ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning));
#endif
}

public List<string> GetFcmAppIdsForWhichNoConfigurationExists(ICollection<string> supportedAppIds)
{
return GetAppIdsForWhichNoConfigurationExists("fcm", supportedAppIds);
Expand Down Expand Up @@ -147,15 +165,6 @@ public override int SaveChanges(bool acceptAllChangesOnSuccess)
return result;
}

public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = new())
{
var entities = GetChangedEntities();
var result = base.SaveChangesAsync(cancellationToken);
PublishDomainEvents(entities);

return result;
}

public override Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = new())
{
var entities = GetChangedEntities();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ namespace Backbone.Modules.Devices.Infrastructure.Persistence.Repository;

public class IdentitiesRepository : IIdentitiesRepository
{
private readonly DbSet<Identity> _identities;
private readonly IQueryable<Identity> _readonlyIdentities;
private readonly DevicesDbContext _dbContext;
private readonly DbSet<Device> _devices;
private readonly IQueryable<Device> _readonlyDevices;
private readonly UserManager<ApplicationUser> _userManager;
private readonly DbSet<Identity> _identities;
private readonly DbSet<IdentityDeletionProcessAuditLogEntry> _identityDeletionProcessAuditLogs;
private readonly IQueryable<Device> _readonlyDevices;
private readonly IQueryable<Identity> _readonlyIdentities;
private readonly IQueryable<IdentityDeletionProcessAuditLogEntry> _readonlyIdentityDeletionProcessAuditLogs;
private readonly UserManager<ApplicationUser> _userManager;

public IdentitiesRepository(DevicesDbContext dbContext, UserManager<ApplicationUser> userManager)
{
Expand All @@ -41,6 +41,7 @@ public IdentitiesRepository(DevicesDbContext dbContext, UserManager<ApplicationU
{
return await (track ? _identities : _readonlyIdentities)
.IncludeAll(_dbContext)
.AsSplitQuery()
.FirstWithAddressOrDefault(address, cancellationToken);
}

Expand Down Expand Up @@ -122,6 +123,7 @@ public async Task<DbPaginationResult<Device>> FindAllDevicesOfIdentity(IdentityA
{
var query = _readonlyDevices
.IncludeAll(_dbContext)
.AsSplitQuery()
.OfIdentity(identity);

if (ids.Any())
Expand All @@ -134,6 +136,7 @@ public async Task<DbPaginationResult<Device>> FindAllDevicesOfIdentity(IdentityA
{
return await (track ? _devices : _readonlyDevices)
.IncludeAll(_dbContext)
.AsSplitQuery()
.FirstOrDefaultAsync(d => d.Id == deviceId, cancellationToken);
}

Expand Down Expand Up @@ -170,6 +173,7 @@ public async Task<IEnumerable<Identity>> Find(Expression<Func<Identity, bool>> f
{
return await (track ? _identities : _readonlyIdentities)
.IncludeAll(_dbContext)
.AsSplitQuery()
.Where(filter)
.ToListAsync(cancellationToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ namespace Backbone.Modules.Messages.Infrastructure.Persistence.Database.Reposito

public class MessagesRepository : IMessagesRepository
{
private readonly MessagesDbContext _dbContext;
private readonly DbSet<Message> _messages;
private readonly IQueryable<Message> _readOnlyMessages;
private readonly MessagesDbContext _dbContext;

public MessagesRepository(MessagesDbContext dbContext)
{
Expand All @@ -28,6 +28,7 @@ public async Task<Message> Find(MessageId id, IdentityAddress address, Cancellat
{
var message = await (track ? _messages : _readOnlyMessages)
.IncludeAll(_dbContext)
.AsSplitQuery() // Use split query to avoid cartesian explosion. see: https://learn.microsoft.com/en-us/ef/core/querying/single-split-queries#split-queries
.Where(Message.HasParticipant(address))
.FirstWithId(id, cancellationToken);

Expand All @@ -53,7 +54,8 @@ public async Task<DbPaginationResult<Message>> FindMessagesWithIds(IEnumerable<M
{
var query = (track ? _messages : _readOnlyMessages)
.AsQueryable()
.IncludeAll(_dbContext);
.IncludeAll(_dbContext)
.AsSplitQuery();

if (ids.Any())
query = query.WithIdsIn(ids);
Expand All @@ -78,7 +80,9 @@ public async Task Update(Message message)

public async Task<IEnumerable<Message>> Find(Expression<Func<Message, bool>> expression, CancellationToken cancellationToken)
{
return await _messages.IncludeAll(_dbContext)
return await _messages
.IncludeAll(_dbContext)
.AsSplitQuery()
.Where(expression)
.ToListAsync(cancellationToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public async Task Delete(Expression<Func<Identity, bool>> filter, CancellationTo
{
var identity = await (track ? _identitiesDbSet : _readOnlyIdentities)
.IncludeAll(_dbContext)
.AsSplitQuery()
.FirstOrDefaultAsync(i => i.Address == address, cancellationToken);

return identity;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public async Task<Relationship> FindRelationship(RelationshipId id, IdentityAddr
{
var relationship = await (track ? _relationships : _readOnlyRelationships)
.IncludeAll(_dbContext)
.AsSplitQuery()
.WithParticipant(identityAddress)
.FirstWithId(id, cancellationToken);

Expand All @@ -52,6 +53,7 @@ public async Task<DbPaginationResult<Relationship>> FindRelationshipsWithIds(IEn
var query = (track ? _relationships : _readOnlyRelationships)
.AsQueryable()
.IncludeAll(_dbContext)
.AsSplitQuery()
.WithParticipant(identityAddress)
.WithIdIn(ids);

Expand Down Expand Up @@ -104,6 +106,6 @@ public async Task DeleteRelationships(Expression<Func<Relationship, bool>> filte

public async Task<IEnumerable<Relationship>> FindRelationships(Expression<Func<Relationship, bool>> filter, CancellationToken cancellationToken)
{
return await _relationships.IncludeAll(_dbContext).Where(filter).ToListAsync(cancellationToken);
return await _relationships.IncludeAll(_dbContext).AsSplitQuery().Where(filter).ToListAsync(cancellationToken);
}
}

0 comments on commit 0d73e16

Please sign in to comment.