search

rebus-org / Rebus.SqlServer

:bus: Microsoft SQL Server transport and persistence for Rebus
https://mookid.dk/category/rebus
Other
44 stars 45 forks source link

Change SQL storage transport to not hold transaction open for the duration of event handling #86

Closed kendallb closed 3 years ago

kendallb commented 3 years ago

I was up all night working on the MySqlConnector port of the SQL Server code in this tree, and have it just about completed. I finished up the basic transport code and got the lease transport working last night, but ran into an issue with the DoesNotBarfInTheBackground() test where it deadlocks or processes the message in multiple threads due to the way MySQL does row locking.

It's not possible with MySQL 5.7 and earlier to ignore locked rows in a select statement like you can with SQL Server and (ROWLOCK, READPAST, READCOMMITTEDLOCK). MySQL 8.0 has support for that, but it's recent and we are not using it yet.

So to fix it, I am going to have the change the way the MySQL transport works and make the normal transport similar to the leased transport in that it will need to wait until the transaction scope is either committed or aborted to actually delete the record or free it back up again (the existing MySQL transport tried to do it with the processid GUID, but got it wrong).

Anyway, from my experience with MySQL in production transactions running THAT long are bad, really bad. I am not sure how they work in SQL Server and the tests certainly pass, but I know from our experience running Percona XtraDB cluster for our live databases, long running transactions can cause Percona clustering to throw fits, so we avoid them at all costs. It's possible in a production, heavy loaded SQL Server it would have similar issues?

Anyway, while I will change my code to support earlier MySQL versions without the row locking issues (lease style model), I do wonder if it's not really a good idea even in the SQL Server version?

mookid8000 commented 3 years ago

Anyway, while I will change my code to support earlier MySQL versions without the row locking issues (lease style model), I do wonder if it's not really a good idea even in the SQL Server version?

Could be 🙂 but as you've probably seen, the SQL Server transport can work in either mode, and the transaction-style mode is actually faster than the lease-based mode. So we should probably just leave it alone for now

kendallb commented 3 years ago

Yep, unlike MySQL it does not have deadlock issues doing it that way.