This repository was archived by the owner on Aug 15, 2024. It is now read-only.
Improve MsSql append performance#117
Merged
damianh merged 4 commits intoSQLStreamStore:masterSQLStreamStore/SQLStreamStore:masterfrom May 2, 2018
yreynhout:improve-mssql-appendyreynhout/SqlStreamStore:improve-mssql-appendCopy head branch name to clipboard
Merged
Improve MsSql append performance#117damianh merged 4 commits intoSQLStreamStore:masterSQLStreamStore/SQLStreamStore:masterfrom yreynhout:improve-mssql-appendyreynhout/SqlStreamStore:improve-mssql-appendCopy head branch name to clipboard
damianh merged 4 commits intoSQLStreamStore:masterSQLStreamStore/SQLStreamStore:masterfrom
yreynhout:improve-mssql-appendyreynhout/SqlStreamStore:improve-mssql-appendCopy head branch name to clipboard
Conversation
… 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.
…ream version logic can be simplified.
…he rest of the logic conditional based on that fact (applies to ExpectedVersion.Any/NoStream only).
6733244 to
048efb2
Compare
Member
|
Merged, cheers @yreynhout ! Yeah the duplicate index can be addressed with PR for v2. |
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. |
yreynhout
commented
May 8, 2018
|
|
||
| SELECT @latestStreamVersion = MAX(StreamVersion) + @latestStreamVersion + 1 | ||
| FROM @newMessages | ||
| SET @latestStreamVersion = @latestStreamVersion |
Contributor
Author
There was a problem hiding this comment.
This line can safely be removed - PR inbound.
Member
|
@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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 usedMicrosoft 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.sqlwenew stream messagestable parameter, thereby eliminating the use of an index,positionis PK of themessagestable).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
maxcountof 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):
I performed similar changes for the
AppendStreamExpectedVersionAny.sqlandAppendStreamExpectedVersionNoStream.sql.