NServiceBus.Persistence.Sql icon indicating copy to clipboard operation
NServiceBus.Persistence.Sql copied to clipboard

Possibilities for better performance in MSSQL saga persistence

Open DavidBoike opened this issue 3 years ago • 6 comments

This issue describes possible options for improving saga performance when using MSSQL. These are not necessarily obviously better solutions, and will need to be validated by performance testing with various sized datasets.

ROWLOCK

The queries for get by property and get by id both use the hint with (updlock) and a where clause that should result in only one matching row, either by the primary key (in the case of get by saga id) or by unique index (in the case of get by property).

The assumption is that using the updlock hint will lock only one record, since only one matches the where clause. However it's possible that SQL Server will elect to go with a page or even a table lock, depending on the number of records and probably other factors as well. If it does use page/table locks, that could lock other (perhaps many other) rows for the same saga and have a detrimental effect on concurrent processing and overall performance.

The solution to this would be to use with (updlock, rowlock), however it's also possible that in at least some scenarios too many rowlocks could be detrimental to overall performance as well.

It would be good to test these assumptions in a variety of circumstances and have SQL Persistence do the right thing, or at least provide an API so that the type of lock used can be tuned if necessary.

Covering indexes

A covered index is an index contains all the columns needed to satisfy a query, rather than the current index structure that contains only the correlation property value and requires a bookmark lookup to access the data. This could speed up query times.

However it will also increase storage requirements and potentially increase the time needed to write data to both the clustered index and the covering index.

DavidBoike avatar Mar 10 '22 21:03 DavidBoike

FYI a PR already exists for this:

  • https://github.com/Particular/NServiceBus.Persistence.Sql/pull/376

Second, a user disabled "page locks" which basically results in the same behavior as using the rowlock hint and removed issues.

image

According to the customer the cause is that the query optimizer create a plan based on an empty table which doesn't work once the table contains a lot of saga instances.

ramonsmits avatar Aug 17 '22 09:08 ramonsmits

@ramonsmits Shouldn't users be periodically updating statistics on the SQL server as part of database maintenance in order to prevent faulty query plans from being used?

DavidBoike avatar Aug 17 '22 19:08 DavidBoike

@DavidBoike shouldn't be needed in most cases. I did that in the past myself too to often fix performance issues but in general its because the query was missing the right hints.

ramonsmits avatar Aug 19 '22 12:08 ramonsmits

I disagree, and we need to be careful not to be cavalier with assuming that adding a hint will be the correct general-purpose solution without testing, as I mentioned in the issue description.

DavidBoike avatar Aug 19 '22 13:08 DavidBoike

You never want a page lock for this query. Under no circumstance are there any benefits in that. Issue is that I wouldn't know how you could test this and create a dataset that proves a rowlock is better.

ramonsmits avatar Aug 22 '22 10:08 ramonsmits