Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibilities for better performance in MSSQL saga persistence #898

Open
DavidBoike opened this issue Mar 10, 2022 · 7 comments
Open

Possibilities for better performance in MSSQL saga persistence #898

DavidBoike opened this issue Mar 10, 2022 · 7 comments

Comments

@DavidBoike
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

@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
Copy link
Member

@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
Copy link
Member Author

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
Copy link
Member

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
Copy link

bbrandt commented Sep 6, 2024

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

@ramonsmits
Copy link
Member

@bbrandt In my past experience clustered indexes often perform badly on inserts when the PK isn't chronological as pages will have to be split. When sagas are updated the JSON blob gets changes which can also easily result in page splits. Seems something that we could be testing.

In regards to "where correlation property is an integer,", SQLP will use a non-text type for certain CLR types: https://docs.particular.net/persistence/sql/saga#correlation-ids-correlation-types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants