tompazourek
commented
5 years ago GETDATE vs SYSDATETIMEOFFSET
I also noticed the issue with GETDATE() used instead of SYSDATETIMEOFFSET(). But not really from the performance perspective, but more because it's incorrect.
At the moment, when GETDATE() is used, the timestamps inside the visible and expiration columns actually end up incorrect. The timezone modifier gets lost, and an incorrect timestamp is stored. The SYSDATETIMEOFFSET() is the correct function that should have been used in this transport. Besides being correct with regards to the timezone, and the datetimeoffset column type, it is also more precise.
See the following script for yourself:
SELECT
[CorrectTimestamp] = SYSDATETIMEOFFSET(),
[RebusWrongTimestamp] = CAST(GETDATE() AS DATETIMEOFFSET)I'm currently in +2 (Czechia during daylight saving time, my actual real time is 12:17 PM GMT+2, or in other words 10:17 AM UTC) so I get:
In my case, Rebus stores a timestamp that is two hours in the future (12:17 PM UTC). And it only works because when it compares the timestamps again, it again uses timestamp two hours in the future as the current time.
If we were to switch from GETDATE to SYSDATETIMEOFFSET, it would:
- make the timestamps correct with relation to timezones,
- increase the precision,
- increase the performance (due to avoided type conversions),
- but it would be a breaking change.
The breaking change aspect is that if the SQL Server is running in a different timezone than UTC, and the transport was updated, the messages it has in the queue might be visible/expired several hours earlier or later depending on the offset.
If this were up to me, I would still go ahead and fix this. I think all the timezone stuff needs to be correct.
Milliseconds instead of seconds
I also spotted a 2nd possible improvement more related to precision. There are several cases where the transport uses DATEADD(ss, ...) (visible, and ttlseconds parameters). It would be better if the values passed would be milliseconds. The deferred messages would get delivered more accurately. With the current second-level precision, it often happens that the deferred messages are executed almost a second earlier than they should be. We should switch to milliseconds. This change, on the other hand, should not cause any issues when implemented, the deferring will only be more precise.
mookid8000
A lot of time is used on sorting, i can see that previously the DESC/ASC on priority has been changed but i suspect the index was never updated. So the query to receive messages has a sort operation accounting for most of its time:
And have this IO stats: Table 'applicationqueue_invoiceindexer'. Scan count 1, logical reads 3725, physical reads 0, read-ahead reads 0, lob logical reads 0, lob physical reads 0, lob read-ahead reads 0. Table 'Worktable'. Scan count 0, logical reads 0, physical reads 0, read-ahead reads 0, lob logical reads 0, lob physical reads 0, lob read-ahead reads 0.
I tinkered a bit and introduced a receive index like this: CREATE INDEX IX_Receive ON TABLENAME (priority DESC, visible ASC, id ASC, expiration)
Giving this plan:
And this IO: Table 'applicationqueue_invoiceindexer'. Scan count 1, logical reads 35, physical reads 0, read-ahead reads 0, lob logical reads 0, lob physical reads 0, lob read-ahead reads 0.
Im sure you can make an even better plan but this was enough to get us flowing a bit better for now.
Another thing that might not have a big impact but will show up as a warning in some DBA tools when analyzing the plan cache:
an implicit conversion is used, this could be avoided if GETDATE() is changed to SYSDATETIMEOFFSET() which i assume will require some changes elsewhere.
I performed my tests on a queue with around 1.000.000 entries on a SQL Server 2012.