Particular / NServiceBus.Persistence.Sql

Native SQL Persistence for NServiceBus
https://docs.particular.net/persistence/sql/
Other
37 stars 27 forks source link

Possibilities for better performance in MSSQL saga persistence #898

Open DavidBoike opened 2 years ago

DavidBoike commented 2 years ago

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.

ramonsmits commented 2 years ago

FYI a PR already exists for this:

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.

DavidBoike commented 2 years ago

@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?

ramonsmits commented 2 years ago

@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.

DavidBoike commented 2 years ago

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.

ramonsmits commented 2 years ago

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.

bbrandt commented 2 months ago

@DavidBoike @ramonsmits Would we also be able to get better performance in cases where correlation property is an integer, by clustering the table instead by the correlation property, possibly making the correlation property the PK? And then having in index from the Guid Id to the integer value PK?