Skip to content
This repository was archived by the owner on Aug 15, 2024. It is now read-only.

Conversation

@yreynhout
Copy link
Contributor

@yreynhout yreynhout commented Apr 12, 2018

While appending a very large number - in reasonable batches - of events (in our case close to 2 million), to a stream, we noticed a performance degradation over time that manifested itself both on Windows (we used Microsoft SQL Server 2017 (RTM) - 14.0.1000.169 (X64)) and Linux (we used Microsoft SQL Server 2017 (RTM-CU5) (KB4092643) - 14.0.3023.8 (X64)). Intrigued we started profiling the SQL statements using SQL Profiler. We noticed that the number of reads performed by SQL Server upon each append started to grow the more events were appended. The query execution plan revealed that some of the SQL statements during the append operation were performing Index Scans instead of Index Seeks.

For AppendStreamExpectedVersion.sql we

  • replaced the query for the latest stream position by a SCOPE_IDENTITY() function call yielding the same result but a lot faster, thereby eliminating the use of an index,
  • replaced the query for the latest stream version by a computation (which could also be passed in as a parameter - just didn't feel like changing the calling C# code at this point) based on the new stream messages table parameter, thereby eliminating the use of an index,
  • replaced the query for the latest json data of the metadata stream (if such a stream exists) by a query that uses the latest position of the metadata stream, thereby replacing an index scan by index seek (since position is PK of the messages table).

The first two changes, which happen within the transaction, do not have a different outcome than the previous version of these SQL statements. The latter change, outside of the transaction, switched from using the sort and top clause to an exact position read which could potentially give you a different result. I doubt this will be a problem since there's no transaction in place to give you any guarantee on the outcome of that query (other than "we think this is the latest message in the metadata stream"). Ultimately, it's being used to feed the code with the maxcount of the stream we're appending to.

Timing our own code we saw the following improvement (appending 2 million events to 1 stream in batches of a 1000 events / batch):

  • Before: 2.068.175ms
  • After: 292.974ms

I performed similar changes for the AppendStreamExpectedVersionAny.sql and AppendStreamExpectedVersionNoStream.sql.

@yreynhout yreynhout changed the title [WIP] Improve MsSql append performance Improve MsSql append performance Apr 28, 2018
… query for latest stream version by computation based on new stream messages. Simplified metadata stream query (now uses index seek instead of scan).
…ts. Applied locks to ExpectedVersion script.
…he rest of the logic conditional based on that fact (applies to ExpectedVersion.Any/NoStream only).
@yreynhout yreynhout force-pushed the improve-mssql-append branch from 6733244 to 048efb2 Compare April 28, 2018 14:03
@damianh damianh self-assigned this May 2, 2018
@damianh damianh added this to the v1.1.2 milestone May 2, 2018
@damianh damianh merged commit f7c3045 into SQLStreamStore:master May 2, 2018
@damianh
Copy link
Member

damianh commented May 2, 2018

Merged, cheers @yreynhout !

Yeah the duplicate index can be addressed with PR for v2.

@yreynhout
Copy link
Contributor Author

@damianh I reran my tests this afternoon and getting numbers like 312.696ms and 424.045ms which seem to be in the lower (expected) range.


SELECT @latestStreamVersion = MAX(StreamVersion) + @latestStreamVersion + 1
FROM @newMessages
SET @latestStreamVersion = @latestStreamVersion
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can safely be removed - PR inbound.

@damianh
Copy link
Member

damianh commented Aug 25, 2018

@yreynhout Could you add your test to the LoadTest project? I know it won't be the same but good to get some more scenarios in there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants